delta icon indicating copy to clipboard operation
delta copied to clipboard

🐛 Avoid marking `--` above Git version in format-patch output as deletion

Open Frederick888 opened this issue 2 years ago • 5 comments

  • [x] Please include the raw text output from git, so that we can reproduce the problem. (You can use git --no-pager to produce the raw text output.)
  • [x] A screenshot of Delta's output is often helpful also.

The output of git format-patch has got a version footer:

--
2.35.1

And the -- here is marked as deletion, which is a bit confusing.

image

It'd be nice if this can be avoided :)

Frederick888 avatar Feb 01 '22 23:02 Frederick888

Hi @Frederick888, thanks. What command are you typically issuing when you use delta to view git format-patch output?

Do you have any thoughts about the right fix here? If we are doing line-by-line parsing then -- is ambiguous, since it could genuinely be the removal of a line containing -.

dandavison avatar Feb 02 '22 00:02 dandavison

Turned out that this is not a footer, but an email signature. The version number is just its default value. I literally have never seen anyone customising this in all the mailing lists I'm subscribed to...

I usually use git format-patch -1 git format-patch <since> etc. I know this is designed for emails but these days I also copy its output to GitHub, Slack and so on. If delta only parses and renders lines by line, this is quite tricky to handle and I haven't got a good idea in my mind (especially when we don't know the size of input). But I guess users like me can git format-patch --no-signature instead or even git config --global format.signature '' if they don't care about emails.

Please feel free to close this issue. And thanks for this awesome project btw :)

Frederick888 avatar Feb 02 '22 07:02 Frederick888

On a second thought, is it possible to process the diff size lines? For example,

From 3683cf5dcf10d032741cc570c9d7c6e20d819f7d Mon Sep 17 00:00:00 2001
From: Frederick Zhang <[email protected]>
Date: Wed, 2 Feb 2022 23:02:34 +1100
Subject: [PATCH] Test

Signed-off-by: Frederick Zhang <[email protected]>
---
 README.md | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 5fc8cb0..f70aa05 100644
--- a/README.md
+++ b/README.md
@@ -1,9 +1,10 @@
+new line
+
 hello

-world
+world1

 foo

 bar

--
--
2.35.1

It'll be something like:

num_lines_old, num_lines_new = 9, 10
counter_old, counter_new = 0, 0

while line = input.read():
    switch line[0]:
        ' ':
            counter_new++
            counter_old++
        '+':
            counter_new++
        '-':
            counter_old++
    render(line)
    if counter_old == num_lines_old && counter_new == num_lines_new:
        # done reading current hunk
        return

new counter stops at the empty line after bar, old counter stops at the second last --. Then we know the last -- above 2.35.1 doesn't really belong to the diff.

Frederick888 avatar Feb 02 '22 12:02 Frederick888

That sounds like potentially a good idea -- it feels a bit slack that we're not making use of the diff hunk line coordinates, and there might be other edge cases that it fixes / simplifies existing code. (If you or anyone else would like to look into it, that would of course be very welcome.)

dandavison avatar Feb 02 '22 13:02 dandavison

(If you or anyone else would like to look into it, that would of course be very welcome.)

We have ARCHITECTURE.md as a starting point for anyone interested in working on the delta codebase.

dandavison avatar Feb 02 '22 13:02 dandavison