* .gitattributes escape character? @ 2010-11-03 12:24 Marc Strapetz 2010-11-03 15:47 ` Nguyen Thai Ngoc Duy 0 siblings, 1 reply; 18+ messages in thread From: Marc Strapetz @ 2010-11-03 12:24 UTC (permalink / raw) To: git 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: .gitattributes escape character? 2010-11-03 12:24 .gitattributes escape character? Marc Strapetz @ 2010-11-03 15:47 ` Nguyen Thai Ngoc Duy 2010-11-03 17:13 ` Marc Strapetz ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Nguyen Thai Ngoc Duy @ 2010-11-03 15:47 UTC (permalink / raw) To: Marc Strapetz; +Cc: git On Wed, Nov 3, 2010 at 7:24 PM, Marc Strapetz <marc.strapetz@syntevo.com> wrote: > 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. -- Duy ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: .gitattributes escape character? 2010-11-03 15:47 ` Nguyen Thai Ngoc Duy @ 2010-11-03 17:13 ` Marc Strapetz 2010-11-03 21:03 ` Kevin Ballard 2010-11-04 13:55 ` [PATCH] attr: support quoting pathname patterns in C style Nguyễn Thái Ngọc Duy 2 siblings, 0 replies; 18+ messages in thread From: Marc Strapetz @ 2010-11-03 17:13 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: git >> 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: .gitattributes escape character? 2010-11-03 15:47 ` Nguyen Thai Ngoc Duy 2010-11-03 17:13 ` Marc Strapetz @ 2010-11-03 21:03 ` Kevin Ballard 2010-11-04 13:55 ` [PATCH] attr: support quoting pathname patterns in C style Nguyễn Thái Ngọc Duy 2 siblings, 0 replies; 18+ messages in thread From: Kevin Ballard @ 2010-11-03 21:03 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: Marc Strapetz, git On Nov 3, 2010, at 8:47 AM, Nguyen Thai Ngoc Duy wrote: > On Wed, Nov 3, 2010 at 7:24 PM, Marc Strapetz <marc.strapetz@syntevo.com> wrote: >> 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] attr: support quoting pathname patterns in C style 2010-11-03 15:47 ` Nguyen Thai Ngoc Duy 2010-11-03 17:13 ` Marc Strapetz 2010-11-03 21:03 ` Kevin Ballard @ 2010-11-04 13:55 ` Nguyễn Thái Ngọc Duy 2010-11-04 17:21 ` Sverre Rabbelier ` (2 more replies) 2 siblings, 3 replies; 18+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2010-11-04 13:55 UTC (permalink / raw) To: git, Junio C Hamano, Marc Strapetz; +Cc: 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 <pclouds@gmail.com> --- 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(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index c80ca5d..bc6c65c 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -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: diff --git a/attr.c b/attr.c index 6aff695..f3063d8 100644 --- a/attr.c +++ b/attr.c @@ -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; } diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 25205ac..a57f358 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -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 && -- 1.7.3.2.210.g045198 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-04 13:55 ` [PATCH] attr: support quoting pathname patterns in C style Nguyễn Thái Ngọc Duy @ 2010-11-04 17:21 ` Sverre Rabbelier 2010-11-04 22:53 ` Eric Sunshine 2010-11-05 16:58 ` Junio C Hamano 2 siblings, 0 replies; 18+ messages in thread From: Sverre Rabbelier @ 2010-11-04 17:21 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Marc Strapetz Heya, 2010/11/4 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>: > 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). -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-04 13:55 ` [PATCH] attr: support quoting pathname patterns in C style Nguyễn Thái Ngọc Duy 2010-11-04 17:21 ` Sverre Rabbelier @ 2010-11-04 22:53 ` Eric Sunshine 2010-11-05 2:02 ` Nguyen Thai Ngoc Duy 2010-11-05 16:58 ` Junio C Hamano 2 siblings, 1 reply; 18+ messages in thread From: Eric Sunshine @ 2010-11-04 22:53 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Marc Strapetz On 11/4/2010 9:55 AM, Nguyễn Thái Ngọc Duy wrote: > 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<pclouds@gmail.com> > diff --git a/attr.c b/attr.c > index 6aff695..f3063d8 100644 > --- a/attr.c > +++ b/attr.c > name = 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-04 22:53 ` Eric Sunshine @ 2010-11-05 2:02 ` Nguyen Thai Ngoc Duy 0 siblings, 0 replies; 18+ messages in thread From: Nguyen Thai Ngoc Duy @ 2010-11-05 2:02 UTC (permalink / raw) To: Eric Sunshine; +Cc: git, Junio C Hamano, Marc Strapetz 2010/11/5 Eric Sunshine <ericsunshine@gmail.com>: > On 11/4/2010 9:55 AM, Nguyễn Thái Ngọc Duy wrote: >> >> 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<pclouds@gmail.com> >> diff --git a/attr.c b/attr.c >> index 6aff695..f3063d8 100644 >> --- a/attr.c >> +++ b/attr.c >> name = 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. -- Duy ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-04 13:55 ` [PATCH] attr: support quoting pathname patterns in C style Nguyễn Thái Ngọc Duy 2010-11-04 17:21 ` Sverre Rabbelier 2010-11-04 22:53 ` Eric Sunshine @ 2010-11-05 16:58 ` Junio C Hamano 2010-11-05 21:46 ` Kevin Ballard ` (2 more replies) 2 siblings, 3 replies; 18+ messages in thread From: Junio C Hamano @ 2010-11-05 16:58 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, Marc Strapetz Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > 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 <pclouds@gmail.com> > --- > 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-05 16:58 ` Junio C Hamano @ 2010-11-05 21:46 ` Kevin Ballard 2010-11-08 18:40 ` Junio C Hamano 2010-11-06 8:28 ` Marc Strapetz 2010-11-07 8:05 ` Nguyễn Thái Ngọc Duy 2 siblings, 1 reply; 18+ messages in thread From: Kevin Ballard @ 2010-11-05 21:46 UTC (permalink / raw) To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git, Marc Strapetz On Nov 5, 2010, at 9:58 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > >> 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 <pclouds@gmail.com> >> --- >> 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. 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-05 21:46 ` Kevin Ballard @ 2010-11-08 18:40 ` Junio C Hamano 2010-11-08 21:56 ` Kevin Ballard 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2010-11-08 18:40 UTC (permalink / raw) To: Kevin Ballard; +Cc: Nguyễn Thái Ngọc Duy, git, Marc Strapetz Kevin Ballard <kevin@sb.org> writes: > 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-08 18:40 ` Junio C Hamano @ 2010-11-08 21:56 ` Kevin Ballard 2010-11-09 7:48 ` Johannes Sixt 2010-11-10 0:07 ` Junio C Hamano 0 siblings, 2 replies; 18+ messages in thread From: Kevin Ballard @ 2010-11-08 21:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git, Marc Strapetz On Nov 8, 2010, at 10:40 AM, Junio C Hamano wrote: > Kevin Ballard <kevin@sb.org> writes: > >> 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-08 21:56 ` Kevin Ballard @ 2010-11-09 7:48 ` Johannes Sixt 2010-11-09 8:08 ` Kevin Ballard 2010-11-10 0:07 ` Junio C Hamano 1 sibling, 1 reply; 18+ messages in thread From: Johannes Sixt @ 2010-11-09 7:48 UTC (permalink / raw) To: Kevin Ballard Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy, git, Marc Strapetz Am 11/8/2010 22:56, schrieb Kevin Ballard: > 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-09 7:48 ` Johannes Sixt @ 2010-11-09 8:08 ` Kevin Ballard 0 siblings, 0 replies; 18+ messages in thread From: Kevin Ballard @ 2010-11-09 8:08 UTC (permalink / raw) To: Johannes Sixt Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy, git, Marc Strapetz On Nov 8, 2010, at 11:48 PM, Johannes Sixt wrote: > Am 11/8/2010 22:56, schrieb Kevin Ballard: >> 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. 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-08 21:56 ` Kevin Ballard 2010-11-09 7:48 ` Johannes Sixt @ 2010-11-10 0:07 ` Junio C Hamano 2010-11-10 0:27 ` Kevin Ballard 1 sibling, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2010-11-10 0:07 UTC (permalink / raw) To: Kevin Ballard; +Cc: Nguyễn Thái Ngọc Duy, git, Marc Strapetz Kevin Ballard <kevin@sb.org> writes: > 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? > ... 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". ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-10 0:07 ` Junio C Hamano @ 2010-11-10 0:27 ` Kevin Ballard 0 siblings, 0 replies; 18+ messages in thread From: Kevin Ballard @ 2010-11-10 0:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git, Marc Strapetz On Nov 9, 2010, at 4:07 PM, Junio C Hamano wrote: > Kevin Ballard <kevin@sb.org> writes: > >> 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. >> ... 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. > 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. > 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] attr: support quoting pathname patterns in C style 2010-11-05 16:58 ` Junio C Hamano 2010-11-05 21:46 ` Kevin Ballard @ 2010-11-06 8:28 ` Marc Strapetz 2010-11-07 8:05 ` Nguyễn Thái Ngọc Duy 2 siblings, 0 replies; 18+ messages in thread From: Marc Strapetz @ 2010-11-06 8:28 UTC (permalink / raw) To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git >> 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. On 05.11.2010 17:58, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > >> 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 <pclouds@gmail.com> >> --- >> 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. > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] attr: support quoting pathname patterns in C style 2010-11-05 16:58 ` Junio C Hamano 2010-11-05 21:46 ` Kevin Ballard 2010-11-06 8:28 ` Marc Strapetz @ 2010-11-07 8:05 ` Nguyễn Thái Ngọc Duy 2 siblings, 0 replies; 18+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2010-11-07 8:05 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Marc Strapetz Full 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. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- On Fri, Nov 05, 2010 at 09:58:46AM -0700, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > > > 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 <pclouds@gmail.com> > > --- > > 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(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index c80ca5d..f7954dd 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -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: diff --git a/attr.c b/attr.c index 6aff695..fdc4aae 100644 --- a/attr.c +++ b/attr.c @@ -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; } diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 25205ac..7c55482 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -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 && -- 1.7.3.2.210.g045198 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2010-11-10 0:27 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-03 12:24 .gitattributes escape character? Marc Strapetz 2010-11-03 15:47 ` Nguyen Thai Ngoc Duy 2010-11-03 17:13 ` Marc Strapetz 2010-11-03 21:03 ` Kevin Ballard 2010-11-04 13:55 ` [PATCH] attr: support quoting pathname patterns in C style Nguyễn Thái Ngọc Duy 2010-11-04 17:21 ` Sverre Rabbelier 2010-11-04 22:53 ` Eric Sunshine 2010-11-05 2:02 ` Nguyen Thai Ngoc Duy 2010-11-05 16:58 ` Junio C Hamano 2010-11-05 21:46 ` Kevin Ballard 2010-11-08 18:40 ` Junio C Hamano 2010-11-08 21:56 ` Kevin Ballard 2010-11-09 7:48 ` Johannes Sixt 2010-11-09 8:08 ` Kevin Ballard 2010-11-10 0:07 ` Junio C Hamano 2010-11-10 0:27 ` Kevin Ballard 2010-11-06 8:28 ` Marc Strapetz 2010-11-07 8:05 ` Nguyễn Thái Ngọc Duy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).