Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:58:38

Nguyễn Thái Ngọc Duy  [off-list ref] writes:
quoted hunk
:(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 <redacted>
---
 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help