Re: [PATCH 1/3] pathspec: catch prepending :(prefix) on pathspec with short magic

2013-09-05 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 :(prefix) is in the long form. Suppose people pass :!foo with '!'
 being the short form of magic 'bar', the code will happily turn it to
 :(prefix..)!foo, which makes '!' part of the path and no longer a magic.

 The correct form must be ':(prefix..,bar)foo', but as so far we
 haven't had any magic in short form yet (*), the code to convert from
 short form to long one will be inactive anyway. Let's postpone it
 until a real short form magic appears.

 (*) The short form magic '/' is a special case and won't be caught by
 this die(), which is correct. When '/' magic is detected, prefixlen is
 set back to 0 and the whole if (prefixlen..) block is skipped.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  fixes on top of nd/magic-pathspec.

  pathspec.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/pathspec.c b/pathspec.c
 index d9f4143..62fde50 100644
 --- a/pathspec.c
 +++ b/pathspec.c
 @@ -231,7 +231,9 @@ static unsigned prefix_pathspec(struct pathspec_item 
 *item,
   const char *start = elt;
   if (prefixlen  !literal_global) {
   /* Preserve the actual prefix length of each pattern */
 - if (long_magic_end) {
 + if (short_magic)
 + die(BUG: prefixing on short magic is not 
 supported);
 + else if (long_magic_end) {
   strbuf_add(sb, start, long_magic_end - start);
   strbuf_addf(sb, ,prefix:%d, prefixlen);
   start = long_magic_end;

Good.

I wonder if we should start thinking about removing the big
NEEDSWORK comment in front of this function.

Also the pathspec_magic[] array was the table-driven way that was
meant to be enhanced to fit future needs to parse all supported
types of pathspec magic, but it seems that prefix:12 magic is
implemented using a custom/special case code.  We may want to see if
we want to enrich the parser to fold this to match the table-driven
approach better someday---this is not urgent as we are not adding
any new pathspec magic now.

Will queue.  Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] pathspec: catch prepending :(prefix) on pathspec with short magic

2013-09-04 Thread Nguyễn Thái Ngọc Duy
:(prefix) is in the long form. Suppose people pass :!foo with '!'
being the short form of magic 'bar', the code will happily turn it to
:(prefix..)!foo, which makes '!' part of the path and no longer a magic.

The correct form must be ':(prefix..,bar)foo', but as so far we
haven't had any magic in short form yet (*), the code to convert from
short form to long one will be inactive anyway. Let's postpone it
until a real short form magic appears.

(*) The short form magic '/' is a special case and won't be caught by
this die(), which is correct. When '/' magic is detected, prefixlen is
set back to 0 and the whole if (prefixlen..) block is skipped.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 fixes on top of nd/magic-pathspec.

 pathspec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pathspec.c b/pathspec.c
index d9f4143..62fde50 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -231,7 +231,9 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
const char *start = elt;
if (prefixlen  !literal_global) {
/* Preserve the actual prefix length of each pattern */
-   if (long_magic_end) {
+   if (short_magic)
+   die(BUG: prefixing on short magic is not 
supported);
+   else if (long_magic_end) {
strbuf_add(sb, start, long_magic_end - start);
strbuf_addf(sb, ,prefix:%d, prefixlen);
start = long_magic_end;
-- 
1.8.2.83.gc99314b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html