.gitattributes escape character?
Is there an escape character which may be used in .gitattributes to escape e.g. the space-character? Could octal-escaping help here (I didn't succeed)? Thanks for any hints.
Marc.
Quoted 3 lines · Marc Strapetz
Is there an escape character which may be used in .gitattributes to escape e.g. the space-character? Could octal-escaping help here (I didn't succeed)? Thanks for any hints.
You mean escape the path part in .gitattributes? Sorry, no.
I think we can teach git about path quoting though. A leading double quote means the path is quoted, C-style.
Signature · 1 line
Duy
PATCH 1/1 attr: support quoting pathname patterns in C style
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly 'pat"t"ern', not 'pattern'. Also clarify that leading whitespaces are not part of the pattern and document comment syntax.
---
Obvious regression: patterns that begin with double quote will now work differently.
Documentation/gitattributes.txt | 8 +++++--- attr.c | 25 +++++++++++++++++++++---- t/t0003-attributes.sh | 25 ++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 8 deletions(-)
Documentation/gitattributes.txt +5 -3
@@ -21,9 +21,11 @@ Each line in `gitattributes` file is of form: pattern attr1 attr2 ... That is, a pattern followed by an attributes list, -separated by whitespaces. When the pattern matches the -path in question, the attributes listed on the line are given to -the path. +separated by whitespaces. Leading and trailing whitespaces are +ignored. Lines that begin with '#' are ignored. Patterns +that begin with a double quote are quoted in C style. +When the pattern matches the path in question, the attributes +listed on the line are given to the path. Each attribute can be in one of these states for a given path:
attr.c +21 -4
@@ -2,6 +2,7 @@#include "cache.h" #include "exec_cmd.h" #include "attr.h" +#include "quote.h" const char git_attr__true[] ="(builtin)true" ;const char git_attr__false[] ="\0(builtin)false" ;
@@ -181,21 +182,33 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, {int namelen;int num_attr; -const char *cp, *name; +const char *cp, *name, *ep;struct match_attr *res =NULL ;int pass;int is_macro; +struct strbuf pattern =STRBUF_INIT ; cp = line +strspn (line, blank);if (!*cp || *cp =='#' )return NULL ; name = cp; - namelen =strcspn (name, blank); +if (*cp =='"' ) { +if (unquote_c_style (&pattern, name, &ep)) +die ("Broken attribute line: %s" , line); + namelen = ep - name; + name = pattern.buf; + } +else { + namelen =strcspn (name, blank); + ep = name + namelen; + } +if (strlen (ATTRIBUTE_MACRO_PREFIX ) < namelen && !prefixcmp (name,ATTRIBUTE_MACRO_PREFIX )) {if (!macro_ok) {fprintf (stderr,"%s not allowed: %s:%d\n" , name, src, lineno); +strbuf_release (&pattern);return NULL ; } is_macro =1 ;
@@ -206,6 +219,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,fprintf (stderr,"%.*s is not a valid attribute name: %s:%d\n" , namelen, name, src, lineno); +strbuf_release (&pattern);return NULL ; } }
@@ -215,12 +229,14 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,for (pass =0 ; pass <2 ; pass++) {/* pass 0 counts and allocates, pass 1 fills */ num_attr =0 ; - cp = name + namelen; + cp = ep; cp = cp +strspn (cp, blank);while (*cp) { cp =parse_attr (src, lineno, cp, &num_attr, res); -if (!cp) +if (!cp) { +strbuf_release (&pattern);return NULL ; + } }if (pass)break ;
@@ -238,6 +254,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, res->is_macro = is_macro; res->num_attr = num_attr; } +strbuf_release (&pattern);return res; }
t/t0003-attributes.sh +24 -1
@@ -10,17 +10,37 @@ attr_check () { expect="$2" git check-attr test -- "$path" >actual && - echo "$path: test: $2" >expect && + echo "$path: test: $expect" >expect && test_cmp expect actual } +attr_check_quote () { + + path="$1" + quoted_path="$2" + expect="$3" + + git check-attr test -- "$path" >actual && + echo "\"$quoted_path\": test: $expect" >expect && + test_cmp expect actual + +} + +test_expect_success 'open-quoted pathname' ' + echo "\"a test=a" >.gitattributes && + test_must_fail attr_check a a +' + test_expect_success 'setup' ' mkdir -p a/b/d a/c && ( echo "[attr]notest !test" + echo "\" d \" test=d" + echo " e test=e" + echo " e\" test=e" echo "f test=f" echo "a/i test=a/i" echo "onoff test -test"
@@ -44,6 +64,9 @@ test_expect_success 'setup' ' test_expect_success 'attribute test' ' + attr_check " d " d && + attr_check e e && + attr_check_quote e\" e\\\" e && attr_check f f && attr_check a/f f && attr_check a/c/f f &&
Signature · 1 line
1.7.3.2.210.g045198
Re: [PATCH] attr: support quoting pathname patterns in C style
Quoted 9 lines · Nguyễn Thái Ngọc Duy
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly 'pat"t"ern', not 'pattern'. Also clarify that leading whitespaces are not part of the pattern and document comment syntax.
Signed-off-by Nguyễn Thái Ngọc Duy<[email protected]>attr.c +0
-0
attr.c static struct match_attr *parse_attr_line(const char *line, const char *src, · line 197
Quoted 5 lines
attr.c +3
-1name = cp; - namelen =strcspn (name, blank); +if (*cp =='"' ) { +if (unquote_c_style (&pattern, name,&ep)) +die ("Broken attribute line: %s" , line);
The error message is perhaps a bit too generic, indicating only that _something_ on the line caused a problem. Mentioning that bad quoting of 'name' was at fault would help focus the user's attention where it is needed.
-- ES
Re: [PATCH] attr: support quoting pathname patterns in C style
2010/11/5 Eric Sunshine <[email protected]>:
Quoted 6 lines · Nguyễn Thái Ngọc Duy
Quoted 5 lines
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly 'pat"t"ern', not 'pattern'. Also clarify that leading whitespaces are not part of the pattern and document comment syntax.
Quoted 15 lines
Quoted 10 lines
Signed-off-by Nguyễn Thái Ngọc Duy<[email protected]>attr.c +0
-0name = cp;
- namelen =strcspn(name, blank);
+ if (*cp == '"') {
+ if (unquote_c_style(&pattern, name,&ep))
+die("Broken attribute line: %s", line);The error message is perhaps a bit too generic, indicating only that _something_ on the line caused a problem. Mentioning that bad quoting of 'name' was at fault would help focus the user's attention where it is needed.
Really.
Signature · 1 line
Duy
Re: [PATCH] attr: support quoting pathname patterns in C style
In reply to Nguyễn Thái Ngọc DuyHeya,
2010/11/4 Nguyễn Thái Ngọc Duy <[email protected]>:
Quoted 2 lines
Obvious regression: patterns that begin with double quote will now work differently.
It's a behavior change, not a regression (a regression is an _unintended_ behavior change).
Signature · 3 lines
Cheers, Sverre Rabbelier
Re: [PATCH] attr: support quoting pathname patterns in C style
In reply to Nguyễn Thái Ngọc DuyQuoted 8 lines · Nguyễn Thái Ngọc Duy
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly 'pat"t"ern', not 'pattern'. Also clarify that leading whitespaces are not part of the pattern and document comment syntax.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Obvious regression: patterns that begin with double quote will now work differently.
I'm really hesitant to pursue this route and break people's existing setups, especially if the only benefit this patch tries to achieve is to allow somebody to say:
"Program Files/*.txt" ...some attr...
It is not worth the effort, risk and headache, especially because people with such paths are probably already using
Program?Files/*.txt ...some attr..
to match them.
Re: [PATCH] attr: support quoting pathname patterns in C style
Quoted 15 lines · Junio C Hamano
Quoted 8 lines · Nguyễn Thái Ngọc Duy
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly 'pat"t"ern', not 'pattern'. Also clarify that leading whitespaces are not part of the pattern and document comment syntax.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Obvious regression: patterns that begin with double quote will now work differently.I'm really hesitant to pursue this route and break people's existing setups, especially if the only benefit this patch tries to achieve is to allow somebody to say:
Quoted 8 lines
"Program Files/*.txt" ...some attr...It is not worth the effort, risk and headache, especially because people with such paths are probably already using
Program?Files/*.txt ...some attr..to match them.
Would this actually break any existing setups? The only ones that are affected are ones beginning with ", which I imagine would be rather rare. I personally am in favor of having an unambiguous way to encode whitespace into the pattern. Having to use ? has always struck me as being, well, not very good, especially if you have 2 files that only differ at that character (e.g. file.1 and "file 1").
-Kevin Ballard
Re: [PATCH] attr: support quoting pathname patterns in C style
Quoted 2 lines · Kevin Ballard
Would this actually break any existing setups? The only ones that are affected are ones beginning with ", which I imagine would be rather rare.
No regression policy means just that. We try not to break "rather rare" people, only to add support for different kinds of "rather rare" setups, especially when the latter have working workarounds.
Re: [PATCH] attr: support quoting pathname patterns in C style
Quoted 8 lines · Junio C Hamano
Quoted 2 lines · Kevin Ballard
Would this actually break any existing setups? The only ones that are affected are ones beginning with ", which I imagine would be rather rare.
No regression policy means just that. We try not to break "rather rare" people, only to add support for different kinds of "rather rare" setups, especially when the latter have working workarounds.
I'm still not convinced that the workaround is appropriate. If I have a file named "file 1" that needs special attributes, then it is impossible to leave those attributes unspecified for, e.g. "file.1". I'd have to set them on "file 1" and then immediately unset them for "file.1". And as unspecified behaves differently than unset, this can be a problem. Especially because if I add any other files in the future that match "file?1" I have to remember to go through gitattributes and re-set all of the ones already specified to something appropriate for this file.
Sure, this is probably a rare case, but so is filenames beginning with a ".
Basically what I'm trying to say is, we already break one particular "rather rare" setup. I would love to come up with a solution that supports both setups, but I don't know if one exists outside of using a config variable to control whether git attribute patterns support quoting (a solution I am not particularly fond of for this case).
-Kevin Ballard
Re: [PATCH] attr: support quoting pathname patterns in C style
Am 11/8/2010 22:56, schrieb Kevin Ballard:
Quoted 5 lines
Basically what I'm trying to say is, we already break one particular "rather rare" setup. I would love to come up with a solution that supports both setups, but I don't know if one exists outside of using a config variable to control whether git attribute patterns support quoting (a solution I am not particularly fond of for this case).
Can we perhaps have a pseudo-attribute 'quoted-names' that is to be used like this:
* quoted-names
"file 1" binary
file.1 -diff
Its meaning would be that the remainder of the current .gitattributes file is to be parsed with C style path quoting enabled. The glob given with this attribute is irrelevant and ignored.
I didn't check whether old gits would ignore this unknown attribute.
-- Hannes
Re: [PATCH] attr: support quoting pathname patterns in C style
Quoted 14 lines · Johannes Sixt
Am 11/8/2010 22:56, schrieb Kevin Ballard:
Quoted 5 lines
Basically what I'm trying to say is, we already break one particular "rather rare" setup. I would love to come up with a solution that supports both setups, but I don't know if one exists outside of using a config variable to control whether git attribute patterns support quoting (a solution I am not particularly fond of for this case).
Can we perhaps have a pseudo-attribute 'quoted-names' that is to be used like this:
* quoted-names
"file 1" binary
file.1 -diff
Quoted 5 lines
Its meaning would be that the remainder of the current .gitattributes file is to be parsed with C style path quoting enabled. The glob given with this attribute is irrelevant and ignored.
I didn't check whether old gits would ignore this unknown attribute.
This would certainly solve the regression issue, but from the perspective of a brand new user of git this would make absolutely no sense and would be a huge wart upon consistency.
-Kevin Ballard
Re: [PATCH] attr: support quoting pathname patterns in C style
In reply to Kevin BallardQuoted 2 lines · Kevin Ballard
Basically what I'm trying to say is, we already break one particular "rather rare" setup.
Let's try again. One particular "rather rare" setup never worked. As it is "rather rare", we do not really care that deeply to make that work. Another particular "rather rare" setup used to work. Even though we do not really care that deeply to keep it working, is it worth breaking it?
Quoted 4 lines
... I would love to come up with a solution that supports both setups, but I don't know if one exists outside of using a config variable to control whether git attribute patterns support quoting (a solution I am not particularly fond of for this case).
Controlling this with a config would be a disaster. It would mean that the same version of updated git would interpret the same .gitattributes file differently, and the situation will continue forever. Compared to that, the idea J6t brought up would be far easier to swallow. Older vintage of git will misbehave on "rather rare" paths upon seeing a cquoted pattern (i.e. the pattern will not match the intended paths, and will instead match "rather rare" paths that begin with dq) but that is no worse than what we already have. And newer vintages of git will interpret the attribute file written with that magic exactly the same way everywhere, regardless of the configuration setting.
Having said all that, I actually am in favor of using cquote. It would have been what we should have done in the first place.
My preference is to admit that we made a mistake of not using cquote when we originally introduced .gitattributes, clearly state that the version of git with this new backward incompatible feature will _break_ rare existing setups if they had paths whose name begin with a dq and applied attributes to them, and use cquote unconditionally, perhaps with a version bump.
I just didn't like the tone of saying "Nobody would have used such an insane path anyway so we don't care". I am Ok if our message is "Sorry, this release would break if you used to rely on this; we think it is unlikely and are hoping that most of you won't be affected".
Re: [PATCH] attr: support quoting pathname patterns in C style
Quoted 9 lines · Junio C Hamano
Quoted 2 lines · Kevin Ballard
Basically what I'm trying to say is, we already break one particular "rather rare" setup.
Let's try again. One particular "rather rare" setup never worked. As it is "rather rare", we do not really care that deeply to make that work. Another particular "rather rare" setup used to work. Even though we do not really care that deeply to keep it working, is it worth breaking it?
My feeling is yes. My suspicion is that paths that begin with a dq are extremely rare, and ones that have custom git attributes set on them are rarer still. I would rather make the change to support an unambiguous format to specify any possible path and simply apologize if anyone is actually hit by this.
Quoted 8 lines
Quoted 4 lines
... I would love to come up with a solution that supports both setups, but I don't know if one exists outside of using a config variable to control whether git attribute patterns support quoting (a solution I am not particularly fond of for this case).
Controlling this with a config would be a disaster. It would mean that the same version of updated git would interpret the same .gitattributes file differently, and the situation will continue forever.
Precisely why I was not fond of this suggestion.
Quoted 11 lines
Compared to
that, the idea J6t brought up would be far easier to swallow. Older vintage of git will misbehave on "rather rare" paths upon seeing a cquoted pattern (i.e. the pattern will not match the intended paths, and will instead match "rather rare" paths that begin with dq) but that is no worse than what we already have. And newer vintages of git will interpret the attribute file written with that magic exactly the same way everywhere, regardless of the configuration setting.Having said all that, I actually am in favor of using cquote. It would have been what we should have done in the first place.
Agreed.
Quoted 10 lines
My preference is to admit that we made a mistake of not using cquote when we originally introduced .gitattributes, clearly state that the version of git with this new backward incompatible feature will _break_ rare existing setups if they had paths whose name begin with a dq and applied attributes to them, and use cquote unconditionally, perhaps with a version bump.
I just didn't like the tone of saying "Nobody would have used such an insane path anyway so we don't care". I am Ok if our message is "Sorry, this release would break if you used to rely on this; we think it is unlikely and are hoping that most of you won't be affected".
Also agreed. I never meant to imply that we didn't care about paths beginning with a dq, I just thought it was unlikely enough that I would never run into someone using such a path ;)
In any case, I just took a look at Nguyễn's patch and it looks good to me.
-Kevin Ballard
PATCH 1/1 attr: support quoting pathname patterns in C style
In reply to Junio C HamanoFull pattern must be quoted. So 'pat"t"ern attr' will give exactly 'pat"t"ern', not 'pattern'.
If Git fails to unquote it, it warns users and takes the pattern literally. This keeps existing patterns that begin with a double quotation mark work until they get annoyed by the warnings and fix their patterns.
Also clarify that leading whitespaces are not part of the pattern and document comment syntax.
---
Quoted 6 lines · Junio C Hamano
Quoted 4 lines · Nguyễn Thái Ngọc Duy
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly 'pat"t"ern', not 'pattern'. Also clarify that leading whitespaces are not part of the pattern and document comment syntax.
Quoted 7 lines
Quoted 4 lines
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Obvious regression: patterns that begin with double quote will now work differently.I'm really hesitant to pursue this route and break people's existing setups
How about this? No more breaking current setups.
Documentation/gitattributes.txt | 8 +++++--- attr.c | 32 ++++++++++++++++++++++++++++---- t/t0003-attributes.sh | 21 ++++++++++++++++++++- 3 files changed, 53 insertions(+), 8 deletions(-)
Documentation/gitattributes.txt +5 -3
@@ -21,9 +21,11 @@ Each line in `gitattributes` file is of form: pattern attr1 attr2 ... That is, a pattern followed by an attributes list, -separated by whitespaces. When the pattern matches the -path in question, the attributes listed on the line are given to -the path. +separated by whitespaces. Leading and trailing whitespaces are +ignored. Lines that begin with '#' are ignored. Patterns +that begin with a double quotation mark are quoted in C style. +When the pattern matches the path in question, the attributes +listed on the line are given to the path. Each attribute can be in one of these states for a given path:
attr.c +28 -4
@@ -2,6 +2,7 @@#include "cache.h" #include "exec_cmd.h" #include "attr.h" +#include "quote.h" const char git_attr__true[] ="(builtin)true" ;const char git_attr__false[] ="\0(builtin)false" ;
@@ -181,21 +182,40 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, {int namelen;int num_attr; -const char *cp, *name; +const char *cp, *name, *ep;struct match_attr *res =NULL ;int pass;int is_macro; +struct strbuf pattern =STRBUF_INIT ; cp = line +strspn (line, blank);if (!*cp || *cp =='#' )return NULL ; name = cp; - namelen =strcspn (name, blank); +if (*cp =='"' ) { +if (unquote_c_style (&pattern, name, &ep)) { +fprintf (stderr,"Misquoted pattern at %s:%d\n" +"Pattern is taken literally.\n" , + src, lineno); + namelen =strcspn (name, blank); + ep = name + namelen; + } +else { + namelen = ep - name; + name = pattern.buf; + } + } +else { + namelen =strcspn (name, blank); + ep = name + namelen; + } +if (strlen (ATTRIBUTE_MACRO_PREFIX ) < namelen && !prefixcmp (name,ATTRIBUTE_MACRO_PREFIX )) {if (!macro_ok) {fprintf (stderr,"%s not allowed: %s:%d\n" , name, src, lineno); +strbuf_release (&pattern);return NULL ; } is_macro =1 ;
@@ -206,6 +226,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,fprintf (stderr,"%.*s is not a valid attribute name: %s:%d\n" , namelen, name, src, lineno); +strbuf_release (&pattern);return NULL ; } }
@@ -215,12 +236,14 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,for (pass =0 ; pass <2 ; pass++) {/* pass 0 counts and allocates, pass 1 fills */ num_attr =0 ; - cp = name + namelen; + cp = ep; cp = cp +strspn (cp, blank);while (*cp) { cp =parse_attr (src, lineno, cp, &num_attr, res); -if (!cp) +if (!cp) { +strbuf_release (&pattern);return NULL ; + } }if (pass)break ;
@@ -238,6 +261,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, res->is_macro = is_macro; res->num_attr = num_attr; } +strbuf_release (&pattern);return res; }
t/t0003-attributes.sh +20 -1
@@ -10,17 +10,32 @@ attr_check () { expect="$2" git check-attr test -- "$path" >actual && - echo "$path: test: $2" >expect && + echo "$path: test: $expect" >expect && test_cmp expect actual } +attr_check_quote () { + + path="$1" + quoted_path="$2" + expect="$3" + + git check-attr test -- "$path" >actual && + echo "\"$quoted_path\": test: $expect" >expect && + test_cmp expect actual + +} test_expect_success 'setup' ' mkdir -p a/b/d a/c && ( echo "[attr]notest !test" + echo "\"c test=c" + echo "\" d \" test=d" + echo " e test=e" + echo " e\" test=e" echo "f test=f" echo "a/i test=a/i" echo "onoff test -test"
@@ -44,6 +59,10 @@ test_expect_success 'setup' ' test_expect_success 'attribute test' ' + attr_check_quote \"c \\\"c c && + attr_check " d " d && + attr_check e e && + attr_check_quote e\" e\\\" e && attr_check f f && attr_check a/f f && attr_check a/c/f f &&
Signature · 1 line
1.7.3.2.210.g045198
Re: [PATCH] attr: support quoting pathname patterns in C style
In reply to Junio C HamanoQuoted 5 lines
Quoted 2 lines
Obvious regression: patterns that begin with double quote will now work differently.
I'm really hesitant to pursue this route and break people's existing setups
If existing setups are an issue, there could be a config-property "core.gitAttributesQuoting" to enable quoting which will only be set for newly created repositories. Personally, I don't think this effort is necessary. Probably there is not even a single .gitattributes with a leading quotation mark. And if there is, it's easy to fix.
In any case, I think future git repositories and users will be grateful for quoting support: after I noticed problems with a tool-generated(!) .gitattributes files, it took me 5 minutes to try: \-quoting, "-quoting and octal-quoting, but more than 1 hour of googling, looking at git sources and finally writing an email to this list :)
Marc.
Quoted 25 lines · Junio C Hamano
Quoted 8 lines · Nguyễn Thái Ngọc Duy
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly 'pat"t"ern', not 'pattern'. Also clarify that leading whitespaces are not part of the pattern and document comment syntax.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Obvious regression: patterns that begin with double quote will now work differently.I'm really hesitant to pursue this route and break people's existing setups, especially if the only benefit this patch tries to achieve is to allow somebody to say:
"Program Files/*.txt" ...some attr...It is not worth the effort, risk and headache, especially because people with such paths are probably already using
Program?Files/*.txt ...some attr..to match them.
Quoted 8 lines
Quoted 3 lines
Is there an escape character which may be used in .gitattributes to escape e.g. the space-character? Could octal-escaping help here (I didn't succeed)? Thanks for any hints.
You mean escape the path part in .gitattributes? Sorry, no.
I think we can teach git about path quoting though. A leading double quote means the path is quoted, C-style.
But this is not supposed to work already? At least, when quoting the whole path:
"/a file" -text
git check-attr tells me 'file" is not a valid attribute'.
Marc.
Quoted 9 lines · Nguyen Thai Ngoc Duy
Quoted 3 lines · Marc Strapetz
Is there an escape character which may be used in .gitattributes to escape e.g. the space-character? Could octal-escaping help here (I didn't succeed)? Thanks for any hints.
You mean escape the path part in .gitattributes? Sorry, no.
I think we can teach git about path quoting though. A leading double quote means the path is quoted, C-style.
I agree that gitattributes needs to learn about C-style quoting. However, in the meantime you can just replace a space with ? as it's actually a pattern. In other words, "test/file with spaces" can be represented as test/file?with?spaces
-Kevin Ballard