🚀 empty line handling and getting rid of the space leftover from -/+

[andrei-pavel]

Hi, I use delta as part of git in the terminal. I find it really useful to be able to copy chunks of deleted or added code and paste them in my IDE. Problem is there is always one space before each row. So I have to copy, paste and then remove that space for each line. Is it possible to get rid of this one-character column altogether in order to streamline the copy&paste? As default behavior or as an option?

[dandavison]

Hi @andrei-pavel, thanks for this. This has been bothering me too. I think you're right, that we should get rid of them. By default, I think. We have --keep-plus-minus-markers for when people want or need to keep the output similar to git. I'd be interested to hear any counter-arguments to dropping the space in the first column by default.

[phillipwood]

If you drop the space in the first column it is impossible to tell the difference between empty context lines, insertions or deletions as you lose the coloring because the line is empty. I stopped using diff-so-fancy because of this.

On a related note git also marks trailing newlines by coloring background of the + red, if delta wants to highlight whitespace errors then it would need to come up with another way of doing it if it drops the first column.

[FranklinYu]

GNU diff also has that space:

$ cat a.txt
line 1

line 3
$ cat b.txt
line 1

line 3.1
$ diff --unified a.txt b.txt
--- a.txt	2020-06-11 19:53:19.000000000 -0700
+++ b.txt	2020-06-11 19:53:27.000000000 -0700
@@ -1,3 +1,3 @@
 line 1
 
-line 3
+line 3.1

I think this is the convention of most patch files, so I would recommend to keep the space by default, based on the principle of least surprise.

[dandavison]

@philipwood, @andrei-pavel, @FranklinYu, @mk12 thanks very much for your input on this subject. I'm working on this and trying to reach a clear understanding. Let me try to summarize. If you happen to have time to add any further thoughts on the options and default behavior delta should provide that would be really useful and appreciated!

There are two cases:

1. Delta is applying a background color.
   In that case everything is fine currently (in master; there was a bug: 🐛 Trailing whitespace in otherwise empty line is not highlighted #212).
   If we get rid of the leading space (or provide an option for doing so) then we need to decide how to ensure that all desired whitespace changes are highlighted. A possibility would be to inject an artificial space and style it (this is what diff-so-fancy does by default (markEmptyLines)).

2. Delta is not applying a background color (e.g. because the user has activated the diff-so-fancy preset with --presets diff-so-fancy (unreleased feature)).
   In this case we need a solution whether we drop the leading space or not. We could style the leading space, or if we've dropped it then inject an artificial space and style that.

The next release of delta (current master) will take configuration from ~/.gitconfig, so I think it may make sense to respect color.diff.whitespace when styling whitespace.

If you drop the space in the first column it is impossible to tell the difference between empty context lines, insertions or deletions as you lose the coloring because the line is empty. I stopped using diff-so-fancy because of this.

@philipwood what's your opinion about diff-so-fancy's solution to this? (markEmptyLines) As I understand things it addresses the issue you describe, right?

On a related note git also marks trailing newlines by coloring background of the + red, if delta wants to highlight whitespace errors then it would need to come up with another way of doing it if it drops the first column.

Injecting an artificial space and styling it appropriately is a possible solution there too, and is what dsf does.

GNU diff also has that space:...I think this is the convention of most patch files, so I would recommend to keep the space by default, based on the principle of least surprise.

@FranklinYu apologies if I'm misunderstanding your argument. GNU diff -u has leading spaces in the first column (in unchanged lines) because it is using the first column for -/+ markers (in changed lines). However, here I have in mind what delta's behavior should be when it is not emitting -/+ markers. (In which case it can never be a valid patch: to get a patch from delta one would use --color-only or ensure that delta is skipped by redirecting git's stdout).

An alternative to the inject-an-artificial-space-and-style-it solution might be to force a background color across the entire line, even though the user doesn't have background colors configured. I'm not saying that's a great solution, just the only other solution that has occurred to me so far. The downside of injecting a real space is that the user might copy it and commit trailing whitespace and it would be delta's fault. But I'm not yet aware of a suitable way to get a terminal emulator to visually highlight these things other than forcing background color across the full line, or injecting a hard space.

[mk12]

An alternative to the inject-an-artificial-space-and-style-it solution might be to force a background color across the entire line, even though the user doesn't have background colors configured. I'm not saying that's a great solution

For my diff-so-fancy-ish setup, I think I'd be happy with entire blank lines using the emph style (because those are the ones that have a background color). Or you could leave it up to the user with a --minus-blank-style and --plus-blank-style pair of options.

[FranklinYu]

The next release of delta (current master) will take configuration from ~/.gitconfig

Are you reading the file with Git, or parse them yourself? Just making sure that you are aware of multiple possible configuration files. I’m using ~/.config/git/config FYI.

[phillipwood]

1. **Delta is applying a background color.**
   In that case everything is fine currently (in master; there was a bug: #212).
   If we get rid of the leading space (or provide an option for doing so) then we need to decide how to ensure that all desired whitespace changes are highlighted. A possibility would be to inject an artificial space and style it (this is what diff-so-fancy does by default (`markEmptyLines`)).

I'm not sure there is anything todo to style any whitespace changes in this case. If the first character (+- ) is removed the content of the line is unchanged and any whitespace should be styled correctly I think. The thing that annoys me about markEmptyLines is that it adds spaces to empty lines and makes the deletion of an empty line look the same as the deletion of a line with a single space that has been highlighted by diff-highlight - with delta's background coloring that should work without messing with the contents of the line.

2. **Delta is not applying a background color** (e.g. because the user has activated the diff-so-fancy preset with `--presets diff-so-fancy` (unreleased feature)).
   In this case we need a solution whether we drop the leading space or not. We could style the leading space, or if we've dropped it then inject an artificial space and style that.

The next release of delta (current master) will take configuration from ~/.gitconfig, so I think it may make sense to respect color.diff.whitespace when styling whitespace.

If you drop the space in the first column it is impossible to tell the difference between empty context lines, insertions or deletions as you lose the coloring because the line is empty. I stopped using diff-so-fancy because of this.

@philipwood what's your opinion about diff-so-fancy's solution to this? (markEmptyLines) As I understand things it addresses the issue you describe, right?

As I said above I think using background coloring solves this in delta, I'm not in favor of adding extra spaces as markEmptyLines does. As you say below one reason for removing the leading +- is to make it easier to copy and paste from the diff - that is made more difficult if delta starts adding fake spaces.

On a related note git also marks trailing newlines by coloring background of the + red, if delta wants to highlight whitespace errors then it would need to come up with another way of doing it if it drops the first column.

Injecting an artificial space and styling it appropriately is a possible solution there too, and is what dsf does.

Is there a way to do that without injecting a fake space? Maybe there could be a distinct background color to show an added empty line at the end of a file.

An alternative to the inject-an-artificial-space-and-style-it solution might be to force a background color across the entire line, even though the user doesn't have background colors configured. I'm not saying that's a great solution, just the only other solution that has occurred to me so far. The downside of injecting a real space is that the user might copy it and commit trailing whitespace and it would be delta's fault. But I'm not yet aware of a suitable way to get a terminal emulator to visually highlight these things other than forcing background color across the full line, or injecting a hard space.

I think you can color just the first character by doing something like printf '\033[41m\033[1K\033[1D\033[m\n' which sets the background color and deletes from the cursor to the beginning of the line which highlights the first character and then clears the background color and prints a new line. I think it is important to use a distinct color for whitespace errors so they do not get confused with the highlighting for empty lines.

To style the first character differently to the rest of the line without adding a fake space you can do printf '\033[41m\033[1K\033[1C\033[42m\033[K\033[m\n' Which moves the cursor on character to the right after deleting from the cursor to the beginning of the line and then styles the rest of the line as delta already does. That would mean adding C to LESSANSIENDCHARS

[phillipwood]

The next release of delta (current master) will take configuration from ~/.gitconfig

Are you reading the file with Git, or parse them yourself? Just making sure that you are aware of multiple possible configuration files. I’m using ~/.config/git/config FYI.

It's using libgit2 so should be fine and hopefully handle all the complexities of conditional includes and worktree config file.

[dandavison]

@phillipwood thanks very much for your extremely helpful post above. I'll move forwards with trying to style the things we're discussing appropriately without introducing fake characters, using the terminal emulator techniques you point out.

[dandavison]

And, without any leading space: genuine code to start in the first column. Please correct me if this is wrong but my reading of the conversation so far is that we don't have reasons to keep that, and we do have a good reason to get rid of it (copying code from the diff).

[phillipwood]

And, without any leading space: genuine code to start in the first column. Please correct me if this is wrong but my reading of the conversation so far is that we don't have reasons to keep that, and we do have a good reason to get rid of it (copying code from the diff).

Yes I agree

[dandavison]

Thanks very much everyone for the help here. I believe that everything here is addressed in master. If you are able to build and try out master, that is of course very useful.

Copying the PR description from #225:

---

• Unless --keep-plus-minus-markers is in effect, drop the first column so that the real code starts in the first column.

• Highlight addition of trailing whitespace as a whitespace error (i.e. git's blank-at-eol). Style determined by new option --whitespace-error-style which defaults to git's color.diff.whitespace.

• Highlight removals and additions of empty lines with background color in the first column if these would not otherwise be visible (i.e. if the applicable style lacks a background color). Style determined by new options --minus-empty-line-marker-style and --plus-empty-line-marker-style.

• Additions of empty lines at end-of-file are not currently highlighted as whitespace errors, but rather as normal addition-of-empty line. This is contra git's blank-at-eof and is a bug to be addressed in subsequent work.

See git documentation: https://git-scm.com/docs/git-config#Documentation/git-config.txt-corewhitespace