[RFD] Strange patch formats (aka tricks with unified diffs)..

I'm not sure how many people on this list have worked with unified diffs for as long as I have, and are so used to them that they can edit them up (and fix up line numbers etc) in their sleep.

So I don't know if people are used to some of the really nice tricks you can do with them.

For example, a lot of traditional context diff patch proponents (you're wrong, btw) think that unified diffs are hard to read, because you don't see the "end result". There's a really trivial trick for this, which is to just do

	grep -v '^-'

on the patch, and suddenly you see the patch in a totally different light: you see what the end result is (in a much more readable format than a traditional context diff).

The reverse operation ("grep -v '^+'") also obviously works, but since it just shows the code the way it already is, the only reason to use it tends to be an incorrectly reversed patch, or when you just want to see the old code side-by-side with the new code (which is why some people prefer traditional context diffs, but the reason I hate them is that they break up the old/new vertically in random places, so you can't just have two windows open and compare them side by side).

I'm normally actually very good at doing that "grep" mentally, and don't actually often need to do it explicitly at all, unless the patch is just pretty messy. But sometimes the patch is just confusing enough that it helps to explicitly filter it.

Now, the reason I mention this is that I was just in "gitk", and while I love the gitk graphical revision view, I just noticed (once again) why I tend to hate most GUI programs. I can't do the clever tricks! The gitk diffs are nice and colorized, but the "show just end result" trick just doesn't work.

So I can see two ways to fix this:
 - bug Paul about alternate diff viewing capabilities in gitk. He's cc'd 
   here. A way to make the "diff" pane show just the new one, the old one, 
   or even both side-by-side with some mousy interface?
 - add some actual switch to git diff generation to hide the negative side 
   of a unified diff, and add some way to just make gitk pass that switch 
   in.

I dunno. I realize that it's not just gitk - gitweb, qgit etc don't allow the tricks *either*, but gitk was the one I just hit this with. I ended up just cutting-and-pasting the SHA1 and doing the thing in a terminal with the shell pipeline instead. Am wondering if maybe other people have ideas on this.

Maybe I don't need this often enough for it to matter (as mentioned, I'm so good at doing this in squishy-ware that I read patches _without_ the explicit help most of the time) but I realized that if I do it occasionally, perhaps people who haven't worked with unified diffs quite as much as I have never even realized..

		Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

The switch itself would literally be a one-liner (see below).

However, I suspect what you want is _not_ "turn the negative side completely off", but "make the negative side visually more easily ignorable".

How about displaying the deleted lines with light gray on white background, or something like that?

On 4/12/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:

Currently in qgit you can select a revision + a file and call an external diff viewer, as example Kompare (tweakable from 'Edir->settings->External diff viewer' menu) to view the diff side-by-side, or in any other way the viewer allows, for the selected file.

I like also the idea to hide the negative side but I would rather prefer a togglable command/button instead of a command line argument or a setting, so to allow easy and quick hide/unhide on the same patch.

I will work on it..... ;-)

Thanks for idea Marco

On 4/13/07, Marco Costalba <mcostalba@gmail.com> wrote:

I have pushed a patch with support for toggling view of the negative side of an unified diff (right most 'filter' button in patch viewer tab or CTRL+H shortcut).

In case someone is interested repo is, as usual,

                git://git.kernel.org/pub/scm/qgit/qgit.git

  Marco

P.S: Another two useful shortcuts are the hide/unhide of revision description header ('h' key or from 'View' menu) and hide/unhide of secondary panes ('s' key or from 'View' menu) they work in all the different tabs.

Linus Torvalds writes:

Showing just the old lines, or just the new lines, would be dead easy, since the Tk text widget has an "elide" option on tagged ranges of text that effectively removes them from the display. So I don't even have to change the contents of the text widget, merely the display options associated with the tags that I already have attached to the old and new lines (that's how they get their color).

Side-by-side would be more challenging.

Alternatively I could provide a field where you can put in a filter command to be put between the diff generator and the display parser.

Have you seen what dirdiff can do with displaying diffs? You can expose more context, move old and new lines up and down (but not in a manner which changes the meaning of the diff), and split context lines into old/new pairs, all with the mouse. This lets me rearrange a diff to make it more visually meaningful and understandable. I could import that stuff into gitk, since dirdiff is also written in Tcl/Tk.

Paul.

On 4/13/07, Paul Mackerras <paulus@samba.org> wrote:

<wish>If someone gets started on side-by-side, I'd love to have char-level diff colouring, like xxdiff does. Oh, and chocolate icecream too.</wish>

If there's enough rope to get started, I'll be happy to hack it into place.

cheers,

m

"Martin Langhoff" <martin.langhoff@gmail.com> writes:

Is it different from the --color-words (whose implementation I happen to hate)?

On 4/13/07, Junio C Hamano <junkio@cox.net> wrote:

Didn't know about --color-words (goes away, tests...) no, nothing like that. I don't like --color-words.

To see what I mean, try

   git-show v1.4.4:git-cvsserver.perl  > file1
   git-show v1.5.1:git-cvsserver.perl  > file2
   xxdiff file1 file2

Just press 'n' twice to get to the 2nd "hunk" and you'll see what I mean. Next is 'n', previous is 'p'. It's useful to visualise subtle differences, and to spot commonalities in large changes.

Some aspects of xxdiff UI are horrid but this feature... is great. Probably tries a scan for commonalities from the start and end of the line. And then perhaps from the middle or from each beginning of alpha char sequence.

cheers,

m

Hi,

On Thu, 12 Apr 2007, Junio C Hamano wrote:

I know that we talked about it previously. And strictly, I think it is not the implementation that you hate, but the rules that were implemented.

I'll have a look at what you wrote back then, and try to come up with some sensible alternatives, so you don't have to hate the code so much.

Ciao, Dscho

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

One sensible behaviour would be to show exactly the same output lines as the regular output, but color-code only the words that changed. E.g (the words are capitalized to illustrate instead of colored here):

        diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
        index 6a5fcfd..535214c 100644
        --- a/Documentation/git-cvsserver.txt
        +++ b/Documentation/git-cvsserver.txt
        @@ -125,7 +131,7 @@ By default it uses SQLite databases in the git directory, named
         temporary files in the same directory as the database file on
         write so it might not be enough to grant the users using
         git-cvsserver write access to the database file without granting
        -them ALSO write access to the directory.
        +them write access to the directory, TOO.

         You can configure the database backend with the following
         configuration variables:

On Sat, Apr 14, 2007 at 02:55:13AM -0700, Junio C Hamano wrote:

That works great for highlighting a small change between two very similar lines (and I think that is one use of --color-words). However, it's terrible for the other use of color words, which is showing changes which are not well-adapted to line oriented diffs (such as flowed text).

Try this:

cat >orig <<EOF this is a paragraph that has some text in it and is wrapped at 30 characters EOF

cat >rewrapped <<EOF this is yet another paragraph that has some text in it and is wrapped at 30 characters EOF

git-diff --color orig rewrapped git-diff --color-words orig rewrapped

I think one behavior is best for one situation, and the other behavior for another situation. Perhaps there is room for two modes?

-Peff

On Fri, 13 Apr 2007, Paul Mackerras wrote:

Ok, just a radio button for the patch case for "show patch" vs "show result" would certainly be sufficient for what I normally do. Adding the case "show origin" for completeness (ie filter out the new lines, rather than filtering out the old lines) is probably a good idea, even if I've never used it.

Not a biggie. I've used kdiff3 etc in the past, and they do a good job of it and it "looks cool", but I've never done it on a command line which means that in practice _I_ don't use it. Maybe other people do.

I've never used dirdiff. Sounds a bit like what kdiff3 does.

			Linus

Linus Torvalds writes:

Try this patch, let me know what you think...

Paul.

On Wed, 18 Apr 2007, Paul Mackerras wrote:

Very good, looks great. 

		Linus

Linus Torvalds <torvalds@linux-foundation.org> wrote:

This one would still allow to show the added lines still colored in green. Or do I miss a way to do this? git show --color is not very grep friendly because lines begin with an escape sequence.

On Fri, 13 Apr 2007, Matthias Lederhofer wrote:

Heh. I'm not much for color, so I've never done it. But if you want it, you should be able to just do

	git show --color <commit> |
		grep -v '^[^ ]*-' |
		less -RS

because every line of a diff (and even the explanations, thanks to the indent-by-four rule of git log printing) should either start with a "+/-" *or* they will have a space before they will show up, so using the "no spaces before the -" rule instead of "beginning of line" should work..

So the above will *literally* just remove the "---" line and the actual removal lines of the diff, even in the presense of color coding.

The exception, of course, is:
 - maybe I didn't think it through and I missed some case, and it removes 
   lines it shouldn't.
 - if you make the color coding itself have spaces or something in it 
   you break my assumption (but the vt100 codes should be fine)

Hmm?

		Linus