Remove trailing whitespace #3001
Conversation
iSazonov
left a comment
iSazonov
left a comment
There was a problem hiding this comment.
I was unable to check all the cs files because the browser hangs.
For ps1 files commit GitHub show: +2,324 −2,325 - we lost one line. No idea where and why.
| { | ||
| // The string contains index references to other spots in the string, so we need placeholders so we can compute the offsets. | ||
| // The "<<<<<<<<1,<<<<<<<<2, etc" strings are just placeholders. We'll back-patch them actual values within the header location afterwards. | ||
| const string Header = @"Version:0.9 |
There was a problem hiding this comment.
Please note - change in code is potentially dangerous.
There was a problem hiding this comment.
You're right, there is some small risk here (not this change, it looks fine after looking at how the string is used.)
I'm fine with accepting the risk, this change will simplify reviews in the future when folks use editors that trim trailing whitespace.
There was a problem hiding this comment.
Ok.
May be slightly improve the script to receive a warning in this case in future?
There was a problem hiding this comment.
Maybe possible with heuristics and expect false positives - but I'm not planning to spend time on it.
|
To review changes locally (not in a browser), I find https://hub.github.com/ very useful, e.g. I could review this PR with: or something like that. As for the deleted line, that probably happened when I fixed a merge conflict, I think I may have deleted a blank line. |
|
@lzybkr Thanks for Most likely this script we will use in the future, maybe put it in the |
|
I have not found anything more. Although one question is there. Microsoft.PowerShell.Archive module lives in a separate repository and there is no need in its correction here, isn't it? |
|
Correct, there was no need to include Microsoft.PowerShell.Archive, but it would have been more work to exclude those files. |
4 participants
I was concerned about #2725 changing file encodings and possibly other characters. If the number of changes was small, I would have just reviewed it, but there a ton of changes.
I took the script written by @powercode and modified it some to verify we only deleted spaces and tabs, the result is here.
With this extra check, I found 25 or so files where #2725 would have changed characters we might not have wanted changed. Those files are not included in the PR, I'll fix those in another PR - and at the same time fix those funny characters or fix the file encoding to be true ascii.