Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

On Thu, Jan 24, 2019 at 08:46:54AM -0800, Elijah Newren wrote:
> The critical part of the patch is the few line change to
> show_raw_diff(), the rest is plumbing to set that up.
> 
> This patch was based out of Peff's suggestion[1] to fix diff-tree to
> do what I needed rather than bending fast-export to cover my usecase;
> I've been running with a hacky version of this patch for a while and
> finally cleaned it up.
> 
> [1] https://public-inbox.org/git/20181114071454.GB19904@sigill.intra.peff.net/
> 
> As an alternative, I considered perhaps trying to sell it as a bugfix
> (how often do people use -M, -c, and --raw together and have renames
> in merge commits -- can I just change the format to include the old
> names), but was worried that since diff-tree is plumbing and that the
> format was documented to not include original filename(s), that I'd be
> breaking backward compatibility in an important way for someone and
> thus opted for a new flag to get the behavior I needed.
> 
> I did struggle a bit to come up with a name for the option; if others
> have better suggestions, I'm happy to switch.

Maybe --all-names? You should definitely let other people chime in on
this as well; it should be obvious by now that I'm no expert on naming
things.

> Range-diff:
> 1:  29e9ddf532 = 1:  29e9ddf532 log,diff-tree: add --combined-with-paths options for merges with renames
> 
>  Documentation/diff-format.txt      | 23 +++++++++++++---
>  Documentation/git-diff-tree.txt    |  9 +++++--
>  Documentation/rev-list-options.txt |  5 ++++
>  combine-diff.c                     | 42 ++++++++++++++++++++++++++----
>  diff.h                             |  1 +
>  revision.c                         |  7 +++++
>  revision.h                         |  1 +
>  7 files changed, 78 insertions(+), 10 deletions(-)

I think it might be nice to see a test for this option so that we avoid
breaking it in the future. I'm also curious how this works with -z, and
a test for that would be interesting as well (as well as illustrative of
the format). For example, is it still unambiguous for machine parsing,
even with oddly named files?
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]