On Thu, Mar 29, 2012 at 7:45 AM, Jeff King <peff@xxxxxxxx> wrote:
> 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?
>
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?
>> 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.".
--
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