Re: [PATCH] Support pathspec magic :(exclude) and its short form :-

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

 



On Thu, Nov 21, 2013 at 6:48 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>  We don't have many options that say "negative" in short form.
>>  Either '!', '-' or '~'. '!' is already used for bash history expansion.
>>  ~ looks more like $HOME expansion. Which left me '-'.
>
> I agree with your decision to reject ~, but "!not-this-pattern" is
> very much consistent with the patterns used in .gitignore (and the
> "--exclude <pattern>" option), so avoiding "!" and introducing an
> inconsistent "-" only to appease bash leaves somewhat a funny taste
> in my mouth.

The thing about '!'  is it's history expansion in bash and I suspect
not many people are aware of it. So "git log -- :!something" may
recall the last command that has "something" in it, which is confusing
for those new people and may potentially be dangerous (multiple
command in one line, separated by semicolon). Compared to ":git log --
(exclude)somethign" the worst that could happen is a syntax error
message from bash.

Other than that I'm fine with '!' being the shortcut.

Btw I'm thinking of extending pathspec magic syntax a bit to allow
path completion. Right now the user has to write

git log -- :-Documentation

which does not play well with path completion. I'm thinking of accepting

git log -- :- Documentation

In other words, if there's no path (or pattern) component after the
magic, then the next argument must contain the path. This enables path
completion and I haven't seen any drawbacks yet..

>> @@ -427,6 +430,10 @@ void parse_pathspec(struct pathspec *pathspec,
>>               pathspec->magic |= item[i].magic;
>>       }
>>
>> +     if (nr_exclude == n)
>> +             die(_("There is nothing to exclude from by :(exclude) patterns.\n"
>> +                   "Perhaps you forgot to add either ':/' or '.' ?"));
>
> ;-).

Hey it was originally not there, then I made a mistake of typing "git
log -- :-po" and wondered why it shows nothing. Intuitively, if "git
log" shows every path, then "git log -- :-po" should show every path
except 'po' and the user should not be required to type "git log -- :/
:-po". parse_pathspec() can do that, but it's more work and I'm lazy
so I push that back to the user until they scream :)

>> +enum interesting tree_entry_interesting(const struct name_entry *entry,
>> +                                     struct strbuf *base, int base_offset,
>> +                                     const struct pathspec *ps)
>> +{
>> +     enum interesting positive, negative;
>> +     positive = tree_entry_interesting_1(entry, base, base_offset, ps, 0);
>> +
>> +     /*
>> +      *   #  | positive | negative | result
>> +      * -----+----------+----------+-------
>> +      * 1..4 |   -1     |    *     |  -1
>> +      * 5..8 |    0     |    *     |   0
>> +      *   9  |    1     |   -1     |   1
>> +      *  10  |    1     |    0     |   1
>> +      *  11  |    1     |    1     |   0
>> +      *  12  |    1     |    2     |   0
>> +      *  13  |    2     |   -1     |   2
>> +      *  14  |    2     |    0     |   2
>> +      *  15  |    2     |    1     |   0
>> +      *  16  |    2     |    2     |  -1
>> +      */
>
> Not sure what this case-table means...

Sorry, because tree_entry_interesting_1() returns more than "match or
not", we need to combine the result from positive pathspec with the
negative one to correctly handle all_not_interesting and
all_interesting. This table sums it up. I'll add more explanation in
the next patch.
-- 
Duy
--
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]