Re: Segfault in the attr stack
From: Stefan Beller <hidden>
Date: 2016-06-16 02:19:42
Possibly related (same subject, not in this thread)
- 2016-06-16 · Segfault in the attr stack · Stefan Beller <hidden>
- 2016-06-16 · Re: Segfault in the attr stack · Stefan Beller <hidden>
- 2016-06-16 · Re: Segfault in the attr stack · Junio C Hamano <hidden>
- 2016-06-16 · Re: Segfault in the attr stack · Junio C Hamano <hidden>
- 2016-06-16 · Re: Segfault in the attr stack · Junio C Hamano <hidden>
On Wed, Jun 1, 2016 at 3:11 PM, Junio C Hamano [off-list ref] wrote:
By the way, I just noticed that the <specification> part of the
':(attr:<specification>)' syntax would need to be rethought. In the
.gitattributes file everybody has, we see these lines:
*.[ch] whitespace=indent,trail,space
*.sh whitespace=indent,trail,space
but because comma is a special separator in the pathspec magic
system, we cannot do
$ git status ':(attr:whitespace=indent,trail,space)'Right. In [1] I wrote:
quoted
quoted
+ if (!item->attr_check) + item->attr_check = git_attr_check_alloc();Given more than one "attr" magic, e.g. ":(attr:A=a,attr:B)/path", the check may not be empty when we process the second one; we just extend it without losing the existing contents.That is why I am not super happy with it though. ":(attr:A=a,attr:B)/path", ":(attr:A=a B)/path", are the same for the user as well as in the internal data structures. This "wastes" the white space as a possible convenient separator character, e.g. for multiple values. On the other hand it will be easier to type, specially for many attrs (read submodule groups).
So at that time I thought I had communicated the issue enough and we'd
be fine ignoring it for now. I propose to not escape commas, but use
white spaces instead, i.e.
git status ':(attr:whitespace=indent trail space,attr:label=with
more values)' ':attr(attr:foo:bar)'
would match
* all files that have the whitespace AND the label setting (matching
exactly the values)
* OR foo=bar attribute
This syntax would require to repeat ",attr:" for multiple ANDed attributes,
but would save us from escaped commas, which may be a pain both in parsing as
well as doing the input on a shell?
[1] http://thread.gmane.org/gmane.comp.version-control.git/294989/focus=295016
quoted hunk ↗ jump to hunk
I think we should introduce a quoting mechanism to hide these commas from the pathspec magic splitter, e.g. where attr_value_unquote() would copy string while unquoting some special characters (i.e. at least ' ' and ',' because they are used as syntactic elements in the higher level; there might be others).diff --git a/pathspec.c b/pathspec.c index 0a02255..fb22f28 100644 --- a/pathspec.c +++ b/pathspec.c@@ -132,7 +132,7 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va am->match_mode = MATCH_SET; else { am->match_mode = MATCH_VALUE; - am->value = xstrdup(&attr[attr_len + 1]); + am->value = attr_value_unquote(&attr[attr_len + 1]); if (strchr(am->value, '\\')) die(_("attr spec values must not contain backslashes")); }
If we go by whitespaces, we can implement attr_value_unquote as a `tr " " ","` conceptually, which seems easy. Thanks, Stefan