[chirp_devel] [developer] Add diffs-only mode to file diff utility - #1699

[Dan Smith]

> This comment leaves me quite confused. This new mode that I'm
> adding is _way_ closer in behavior to "real diffs", e.g. unix diff(1) or
> "hg diff" than the dump/diff mode that is already present.

Your description says that "ranges of one or more lines that do not
differ are replaced by a single blank line". That means you're stripping
out all the context lines, right? That would be 'diff -u0' which is not
very useful to humans (IMHO).

> What "real diff" prints all 3500 lines of a file with one or two bytes
> different? That's what the current code does. Not that this dump
> isn't useful, it very much is. And given that it already exists for when
> context is wanted, and that the value of context to diffs in a binary file
> isn't that significant compared to the need to not miss any, this second
> mode seems needed to me.

Yeah, fair enough. I wasn't thinking about the "diff the whole file"
there. I wrote that code, but I don't think I've used it that way even once.

> I simply extended the existing interface which was there before I ever
> heard of chirp. To paraphrase your question: "setting one of them to -1
> and the other to what?". The existing code invokes the whole-image
> dump/diff if either address is -1, and it doesn't matter what the other is,
> because neither value is used as a memory location. In the sense that
> you're complaining about, my -2 behaves exactly the same.

First off, please calm down. Nobody is attacking your character.

Now that I get that you're talking about the whole-file diff scenario, I
see why using -2,-2 makes sense and why it makes sense to skip the
common bits. Your commit message mentions a different diffing mode, but
doesn't say anything about it only being available in the whole-file
diff mode, so hopefully you can see where I was confused. I don't keep
100% context of all the code in my head all the time.

> Almost. So that any range of one or more lines that don't differ
> are replaced by exactly one blank line, which is what I said I was
> doing in bug #1699. See quoted patch comment at the top.

So, no context lines at all, right? If so, that still doesn't seem as
useful to me, personally (although as I said, I don't use this). But,
that's fine.

> It could probably be done in either place. I was already modifying
> show_diff_blob() for the fontsize (#1681). If that will get this accepted
> I can redo the work to move this code to common.simple_diff().

Yeah, I very much think that this should be in simple_diff() itself,
since this is a change to the format of the output of that, not just the
simple colorization routine (model vs. view).

--Dan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 538 bytes
Desc: OpenPGP digital signature
Url : http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20140708/3f00de14/attachment-0001.bin