Re: [PATCH] add--interactive: do not expand pathspecs with ls-files

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 14, 2017, 22:03 UTC
Message-ID: <xmqqd1djh5xl.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170314163024.et2cp5e3imrrrf7k@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 10 quoted lines

> We can improve this by skipping "ls-files" completely, and
> just feeding the original pathspecs to the diff commands.
> This solution was discussed in 2010:
>
>   http://public-inbox.org/git/20100105041438.GB12574@coredump.intra.peff.net/
>
> but at the time the diff code's pathspecs were more
> primitive than those used by ls-files (e.g., they did not
> support globs). Making the change would have caused a
> user-visible regression, so we didn't.

Heh. The change and the reasoning are both obviously correct, but how did you find this? Do you have a pile of "old patches that should be resurrected when time is right" and this was picked out of it, or did you see somebody else hit the same thing recently and then went back to the archive?

Show 14 quoted lines

>   b. If the repository contains filenames with literal wildcard
>      characters (e.g., "foo*"), the original code expanded
>      them via "ls-files" and then fed those wildcard names
>      to "diff-index", which would have treated them as
>      wildcards. This was a bug, which is now fixed (though
>      unless you really go through some contortions with
>      ":(literal)", it's likely that your original pathspec
>      would match whatever the accidentally-expanded wildcard
>      would anyway).
>
>      So this takes us one step closer to working correctly
>      with files whose names contain wildcard characters, but
>      it's likely that others remain (e.g., if "git add -i"
>      feeds the selected paths to "git add").

We didn't run with --literal-pathspecs which was a bug, but I suspect that it didn't exist back then ;-).