Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

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

From: Duy Nguyen <hidden>
Date: 2016-06-15 22:59:17

On Thu, Nov 21, 2013 at 6:48 AM, Junio C Hamano [off-list ref] wrote:
quoted
 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..
quoted
@@ -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 :)
quoted
+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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help