Re: [PATCH 0/9] magic pathspec updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nguyen Thai Ngoc Duy <pclouds@xxxxxxxxx> writes:

> On Tue, May 10, 2011 at 12:51 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> This is a series to update the magic pathspec topic, primarily the area
>> around the empty ':' pathspec.
>
> I think you missed git-commit and git-reset. "git commit --include :"
> may get past parse_and_validate_options() (ie. not dying) but later on
> in prepare_index(), there's no pathspec. I did not check very careful
> though.
>
>> Other than that, I think the current code is probably more or less safe to
>> dogfood with known rough edges.
>
> Agreed.

After re-reading the series one more time (including the ones that are
already in next), I think teaching get_pathspec() about a lone ":" was
probably a bad idea. The only reason we might want to say "there is no
pathspec whatsoever" is when a command wants to limit its operation to
the current subdirectory and it changes behaviour when run at the root
level with and without pathspec (e.g. history simplification).

Note that no such command exists in our vocabulary today, so you need
to imagine "git local-log" that acts like "git log" but by default
shows history simplified to explain only the subdirectory you are
currently in, or something.

Such an application can easily notice that argv[] has only a lone ":"
left and do the same thing it does when there is no pathspec, without
affecting other (existing) users of get_pathspec().

But the thing is, we do not have such an application like "local-log"
now, we know we can easily support such commands when needed without
teaching get_pathspec() about a lone ":", and we can be sure that
existing commands that know get_pathspec() will never give them an
empty pathspec if they feed a non-empty argv[] will not be broken by
not teaching get_pathspec() about a lone ":".

I am leaning toward ripping the lone ":" support from the code in
"next". I would also remove ":(icase)" from "next". It was only meant
to be a POC to show how far we could go only by futzing get_pathspec()
function, and was never meant to be a serious implementation of the
feature. It should be re-done after we do deeper conversion and use
the "struct pathspec" interface not "char **" interface.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]