Re: [PATCH v2] pathspec: advice: long and short forms are incompatible

Administrivia.

If "Stavros Ntentos [off-list ref]" is an
address that is not meant to receive any e-mail, please do not
include it on the Cc line and force those who respond to you to
remove it when replying.

I am trying. However, git-send-email keeps pulling that no-reply address, and
git-send-email does not offer any `--exclude-addresses="*glob*"`-like option.

or even just

	':!(...': cannot mix short and long form pathspec magic

it may be sufficiently clear where the problem is.

I slightly disagree, and prefer the `extra_lookahead_chars`. Just 3 characters
[`:!(`] is a bit too little, and the total message sits below the "you can
disable this message" hint.

The seemingly-stray '; or' at the end aside, I am not sure what this
is trying to say.

See the testcase:

hint: If '(glob)*...' is a valid path, explicitly terminate magic parsing with ':'; or
hint: Disable this message with "git config advice.mixedShortLongMagicPathspec false"

I am segway-ing from the "explicitly stop parsing" to the "disable this message" sentence.

If ':(global,icase)foo' were the exact path the user wants to match
with, then "prefix the whole thing with ":(literal)" would be an
understandable hint, but that is not what you are suggesting.

I am siding with the "user entered this situation by mistake", and not with the
"user is explicitly trying to match a file named `:(global,icase)foo`" side.

Offering a more complete message, will become more complex. I disagree with that.
I can settle by offering examples (mine and yours) in the documentation.

It may be more helpful if, rather than looking at what comes after
'(', we looked at what came before '(' and helped the user write
them out in the longform

I don't see any explicit code in parsing the shortform magics, except:
		/* Special case alias for '!' */
		if (ch == '^') {
			*magic |= PATHSPEC_EXCLUDE;
			continue;
		}
and therefore I would like to avoid such task (although I love well-written
DWIMs-or-close-to-them).

quoted

+static int extra_lookahead_chars = 7;

A few problems:

 - This is not something we want to configure.  It does not need to
   be a variable.

I hate macros, only because I cannot expand or modify them during gdb.
(Suggestions welcome! :-D)

 - This is not something anybody other than the code in the new
   block "if (ch  == '(')" in parse_short_magic() needs to know.  It
   does not need to be a file-scope static.

True, but the message was explicitly referred to with i18n code
specifically targeted for such initialization.

I like code doing the same job, sitting together.
I'd prefer to either move both inside (since no one else will ever
refer to this message either), or keep them as-is.

 - 7 is way too long for warning against something like ":!(glob)",
   no?

GRRRRR C :-p
(I'll push the changes on the next iteration; including the `like glob`
removed, and whatever comes from our discussion.)

But with the above "It may be more helpful" suggestion, notice that
I am deliberately refraining from looking at what comes after '(',
so extra-lookahead may not be necessary after all, and nitpicking
about it may be moot.

Lookahead is simply to inform user what git will do with the current
state of affairs, i.e.:

	git log --oneline --format=%s -- ':!(glob)**/file'

will filter with

	NOT '(glob)**/file'

path (truncated for brevity)