On Wed, Mar 28, 2012 at 03:14:31PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> 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:
> + 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";
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html