indent: fix tab replace on OSX #3060
Conversation
|
@gassmoeller FYI |
|
Looks good, did you test this on your Mac? If so, feel free to merge. |
| sed -e 's/\t/ /g' $f >$f.tmp | ||
| # awkward tab replacement because of OSX sed, do not change onless you test it on OSX | ||
| TAB=$'\t' | ||
| sed -e "s/$TAB/ /g" $f >$f.tmp |
There was a problem hiding this comment.
Does $'\t' yield the string expansion of \t?
I'll note that that just expands tabs into two spaces. That's not what tabs do -- they expand to a number of spaces necessary to get to the next multiple of 8 characters (or whatever you set the tab width to). This is likely going to lead to some ugly text...
There was a problem hiding this comment.
Does
$'\t'yield the string expansion of\t?
It will yield the actual tab character (instead of \ followed by t), see https://stackoverflow.com/a/2623007
I'll note that that just expands tabs into two spaces.
Correct, but this is what is happening on master already (I think you reviewed the patch: #3035). It probably doesn't look pretty, but the point is to fail on the tester if you try to commit files with tabs in them. This PR just makes it work on OSX.
This is likely going to lead to some ugly text...
I already made all changes in #3035 but it is not that bad.
There was a problem hiding this comment.
Yes, I thought about that in #3035 where I did think that it looked ugly. Oh well.
sed is weird on OSX as it doesn't understand \t. Work around this.
|
Looks good. |
sed is weird on OSX as it doesn't understand \t. Work around this.