Re: [PATCH] add -p: skip conflicted paths
From: Erik Faye-Lund <hidden>
Date: 2016-06-15 22:53:27
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH] add -p: skip conflicted paths · Tay Ray Chuan <hidden>
- 2016-06-15 · Re: [PATCH] add -p: skip conflicted paths · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] add -p: skip conflicted paths · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] add -p: skip conflicted paths · Jeff King <hidden>
- 2016-06-15 · Re: [PATCH] add -p: skip conflicted paths · Junio C Hamano <hidden>
On Thu, Mar 29, 2012 at 7:45 AM, Jeff King [off-list ref] wrote:
On Wed, Mar 28, 2012 at 03:14:31PM -0700, Junio C Hamano wrote:quoted
Junio C Hamano [off-list ref] writes:quoted
quoted
Totally untested, but something along this line...Well, probably along that line but not there. I think the patch would be a lot cleaner to keep the part I touched intact, and instead add an extra "ls-files -u" that creates %unmerged hash in the way this patch does, immediately before the last for() loop in the function. And then the loop can use %unmerged hash to filter the elements.That is, something like this.That is way better. Your original one would end up putting every file in the repo into @tracked, which would then be fed on the command-line to "diff-index" and company. I suspect on a large repo that could overflow the command-line limits (plus I recall that at one point we performed way worse on "git diff $(git ls-files)" than we do on "git diff", but that may not be the case anymore). However, I can think of two possible improvements:quoted
+ my %unmerged; + if ($filter_unmerged) { + for (run_cmd_pipe(qw(git ls-files -u --), @ARGV)) { + chomp $_; + if (/^[0-7]+ [0-9a-f]{40} [0-3] (.*)/) { + my $path = unquote_path($1); + $unmerged{$path} = 1; + } + } + if (%unmerged) { + for (sort keys %unmerged) { + print colored $error_color, "ignoring unmerged: $_\n"; + } + } + }I like that we are down to a single ls-files invocation here. But can't we determine from the diff-files output whether an entry is unmerged. In my simple tests, I see that --numstat will show two lines for such an entry. Is that reliable?
Nice. I've observed the same thing (although I've seen three entries, not two). I don't know about the reliability, but I think it kind of makes sense; one entry for both parents, and one for the unmerged working-copy version. Junio, what do you think?
quoted hunk ↗ jump to hunk
quoted
sub patch_update_cmd { - my @all_mods = list_modified($patch_mode_flavour{FILTER}); + my @all_mods = list_modified($patch_mode_flavour{FILTER}, 'filter-unmerged'); my @mods = grep { !($_->{BINARY}) } @all_mods;It seems like a more flexible interface would not be to filter unmerged entries, but rather to mark them as we do with BINARY. And then the caller can do as they please, discarding, printing warnings, etc. Right now, the behavior is to simply skip them. But it would be cool if we could eventually show a useful presentation of the changes and ask to stage them. I know from our past discussions it is not quite as feeding the combined diff to the regular hunk-selector. But I'm sure we can come up with something reasonable. Here's the patch that I came up with to do both of those things. Like I said, I am somewhat unsure of the "detect double mentions as unmerged" rule. But we could still use the "ls-files -u" output to mark the unmerged files. ---diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 8f0839d..6204207 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl@@ -336,7 +336,14 @@ sub list_modified {else { $change = "+$add/-$del"; } - $data{$file}{FILE} = $change; + # If we see it twice, it's unmerged. + if (defined $data{$file}{FILE} && + $data{$file}{FILE} ne 'nothing') { + $data{$file}{FILE} = 'unmerged'; + } + else { + $data{$file}{FILE} = $change; + } if ($bin) { $data{$file}{BINARY} = 1; }@@ -1193,6 +1200,10 @@ sub patch_update_cmd {my @mods = grep { !($_->{BINARY}) } @all_mods; my @them; + print colored $error_color, "ignoring unmerged: $_->{VALUE}\n" + for grep { $_->{FILE} eq 'unmerged' } @mods; + @mods = grep { $_->{FILE} ne 'unmerged' } @mods; + if (!@mods) { if (@all_mods) { print STDERR "Only binary files changed.\n";
I like both the result and the flexibility. There's one minor nit, though: Now "git add -i" says "Only binary files changed.", which isn't true. But this is a simple matter of updating the warning to something like "Only binary or unmerged files changed.".