feat: add newlines option to handle when blank lines are added
#2028
Conversation
Takes any combination of `file`, `hunk-header`, and `diff` to insert a blank line after the filename line, hunk header line, or after each diff hunk respectively. Handles edge cases like omitting both file and hunk-header but requesting newlines after diffs. Ensures output never starts with a blank line (unless git somehow causes it to). Commit boundaries are not handled due to the fact that git will insert a blank line between commits by itself unless --oneline is requested. Closes dandavison#1944, dandavison#1517
|
Hi @LunarLambda, thanks for working on this. I'm aware that there have been several requests for control over blank lines in output. Can we make your new option opt-in (i.e. non-default) so that no tests are affected? |
|
I'm afraid the code touches too many spots to really make it entirely opt-in. I tried to make the default value be as close to the original behaviour as possible but it does change some things like not having the extra blank line at the start of output. Especially because I had to move some code to make the logic look reasonable, e.g. printing the newlines after elements not before them (which is incidentally the only thing that allowed file diffs to have a separating blank line between them) With some guidance I could try to fix the tests but if I try to make the changes completely opt-in the code quality will be much worse I think. Though that said I also took quite some time to understand the logical structure of the state machine and when and how things are printed so maybe you also have a better idea for how to approach this functionality than I |
Takes any combination of
file,hunk-header, anddiffto insert a blank line after the filename line, hunk header line, or after each diff hunk respectively.Handles edge cases like omitting both file and hunk-header but requesting newlines after diffs.
Ensures output never starts with a blank line (unless git somehow causes it to).
Commit boundaries are not handled due to the fact that git will insert a blank line between commits by itself unless --oneline is requested.
Closes #1944, #1517
Note: There are ~70 test failures I have not yet looked at. This PR took ~6 hours of non-stop work. If the maintainers could be so kind, please go through the failing tests and let me know if any of them are legitimate regressions or if they're harmless (e.g., tests expecting extraneous newlines that I have removed). Thank you ^^
From what I can tell,
git log -p,git show -p,git diff, andgit add -p/git restore -pall work. So I believe there are no major functionality regressions.