Re: [PATCH] add -p: skip conflicted paths
- From
Jeff King <peff@peff.net>
- Date
- Mar 29, 2012, 05:45 UTC
- Message-ID
- <20120329054558.GA27604@sigill.intra.peff.net>
- In-Reply-To
- <7v62dotltk.fsf@alter.siamese.dyndns.org>
On Wed, Mar 28, 2012 at 03:14:31PM -0700, Junio C Hamano wrote:
Show 11 quoted lines
> Junio C Hamano <gitster@pobox.com> writes: > > >> 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:
Show 15 quoted lines
> + 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?
> 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";