Re: [PATCH] add--interactive: respect diff.algorithm
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 23, 2013, 19:19 UTC
- Message-ID
- <7v8v20fxee.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20130612184410.GB23890@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
Show 12 quoted lines
>> > +my $diff_algorithm = ($repo->config('diff.algorithm') or 'default');
>> > +
>> > my $use_readkey = 0;
>> > my $use_termcap = 0;
>> > my %term_escapes;
>> > @@ -731,6 +733,9 @@ sub run_git_apply {
>> > sub parse_diff {
>> > my ($path) = @_;
>> > my @diff_cmd = split(" ", $patch_mode_flavour{DIFF});
>> > + if ($diff_algorithm ne "default") {
>> > + push @diff_cmd, "--diff-algorithm=${diff_algorithm}";
>> > + }This is not exactly sanitary for "stash -p", whose DIFF element is defined like so:
'stash' => {
DIFF => 'diff-index -p HEAD',and you will end up appending an option after a non-option argument,
It may happen to be accepted by the command line parser which is overly lax, but we would want to tighten it in the longer term.
As a band-aid, we could do something like the attached patch, but for the longer term, we might need to rethink the way the tweaking of the command line is done by $patch_mode_revision.
-- >8 -- Subject: add -i: add extra options at the right place in "diff" command line
Appending "--diff-algorithm=histogram" at the end of canned command line for various modes of "diff" is correct for most of them but not for "stash" that has a non-option already wired in, like so:
'stash' => {
DIFF => 'diff-index -p HEAD',Appending an extra option after non-option may happen to work due to overly lax command line parser, but that is not something we should rely on. Instead, splice in the extra argument immediately after a '-p' option, which is an option to ask for textual diff output that has to be in all variants.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- git-add--interactive.perl | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 5310959..b50551a 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -730,11 +730,23 @@ sub run_git_apply { return close $fh; } +# The command array must have a single "-p" to ask for output in the +# patch form. Splice additional options immediately after it; we +# should not be randomly appending them, as some of the canned command. +# has non-option argument like HEAD already on it. + +sub splice_diff_options { + my $diff_cmd = shift; + @$diff_cmd = map { + ($_ eq '-p') ? ($_, @_) : $_; + } @$diff_cmd; +} + sub parse_diff { my ($path) = @_; my @diff_cmd = split(" ", $patch_mode_flavour{DIFF}); if (defined $diff_algorithm) { - push @diff_cmd, "--diff-algorithm=${diff_algorithm}"; + splice_diff_options(\@diff_cmd, "--diff-algorithm=${diff_algorithm}"); } if (defined $patch_mode_revision) { push @diff_cmd, $patch_mode_revision;