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.