[PATCH] computil: Fix inconsistent handling of slash-escaped option names

[PATCH] computil: Fix inconsistent handling of slash-escaped option names

[dana]

When writing option specs, you're supposed to be able to escape characters in
the option name that are special to the syntax; as the manual says:

>It is possible for options with a literal ‘+’ or ‘=’ to appear, but that
>character must be quoted, for example ‘-\+’.

This sort of works, but not entirely. It does make the name valid, and the
option does appear correctly as a possibility, but because the slashes are
only skipped when parsing, not actually removed from the final name, the
completion system gets confused when you actually use it. For example:

  % _foo() { _arguments -s : '-\+' -a -b }
  % compdef _foo foo
  % foo -+<TAB>

Because of the -s, it should try to complete -a or -b in the same word.
Instead, it starts a new word, and it behaves as if -+ hasn't been used yet.

Unless i'm missing something, this seems to fix it...

dana


diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index cb1c01042..a98574379 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -1409,7 +1409,7 @@ parse_cadef(char *nam, char **args)
 		     (*p != '=' ||
 		      (p[1] != ':' && p[1] != '[' && p[1] != '-')); p++)
 		if (*p == '\\' && p[1])
-		    p++;
+		    chuck(p);
 
 	    /* The character after the option name specifies the type. */
 	    c = *p;
@@ -1527,7 +1527,7 @@ parse_cadef(char *nam, char **args)
 
 	    opt->next = NULL;
 	    opt->gsname = doset;
-	    opt->name = ztrdup(rembslashcolon(name));
+	    opt->name = ztrdup(name);
 	    if (descr)
 		opt->descr = ztrdup(descr);
 	    else if (adpre && oargs && !oargs->next) {
diff --git a/Test/Y03arguments.ztst b/Test/Y03arguments.ztst
index fa4589374..ec9fd9e8a 100644
--- a/Test/Y03arguments.ztst
+++ b/Test/Y03arguments.ztst
@@ -78,11 +78,17 @@
 >line: {tst rest arg2 }{}
 >line: {tst rest arg2 rest }{}
 
- tst_arguments '-\+[opt]'
+ tst_arguments -s : '-\+[opt1]' '-a[opt2]' '-b[opt3]'
  comptest $'tst -\C-d'
-0:-+
+ comptest $'tst -+\C-d'
+0:slash-escaped option name (-+)
 >DESCRIPTION:{option}
->NO:{-+  -- opt}
+>NO:{-+  -- opt1}
+>NO:{-a  -- opt2}
+>NO:{-b  -- opt3}
+>DESCRIPTION:{option}
+>NO:{-a  -- opt2}
+>NO:{-b  -- opt3}
 
  tst_arguments -+o
  comptest $'tst -\t\t\t\C-w\C-w+\t\t\t'

Re: [PATCH] computil: Fix inconsistent handling of slash-escaped option names

[dana]

On 21 Dec 2018, at 03:41, dana <dana@dana.is> wrote:
>Unless i'm missing something, this seems to fix it...

It does, but it also breaks some behaviour that _describe relies on.
Specifically, it expects comparguments to return the \ escape sequences as-is,
and it doesn't do that any more now that i'm unescaping everything.

The real-world consequences of this are not very noticeable, since _arguments
and _describe don't handle option names containing : and \ well anyway. I
decided it's probably not worth trying to fix that. But i didn't want to leave
anything worse than i found it, so here's a v2 patch.

It simply changes bslashcolon() — which is pretty much only used to re-escape
option names for _describe — to also re-escape \. That should at least keep
the status quo. I thought about renaming the function to bslashbslashcolon(),
but that felt kind of extra, so i didn't.

(Aside from that i also expanded some comments and tests, and removed a
rembslashcolon() that i'd forgot before)

dana


diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index cb1c01042..b54e62aec 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -1060,7 +1060,8 @@ rembslashcolon(char *s)
     return r;
 }
 
-/* Add backslashes before colons. */
+/* Add backslashes before backslashes and colons. This is suitable for e.g.
+ * re-escaping option names to pass back to _describe. */
 
 static char *
 bslashcolon(char *s)
@@ -1070,7 +1071,7 @@ bslashcolon(char *s)
     r = p = zhalloc((2 * strlen(s)) + 1);
 
     while (*s) {
-	if (*s == ':')
+	if (*s == '\\' || *s == ':')
 	    *p++ = '\\';
 	*p++ = *s++;
     }
@@ -1402,14 +1403,19 @@ parse_cadef(char *nam, char **args)
 		return NULL;
 	    }
 
-	    /* Skip over the name. */
+	    /* Skip over the name, unescaping to support unusual option names
+	     * like -+ and -= as described in zshcompsys(1). Note that other
+	     * parts of the completion system have their own considerations for
+	     * certain special characters; for example, the option -: may pose
+	     * issues for utility functions which expect : to introduce a
+	     * description. */
 	    for (p++; *p && *p != ':' && *p != '[' &&
 		     ((*p != '-' && *p != '+') ||
 		      (p[1] != ':' && p[1] != '[')) &&
 		     (*p != '=' ||
 		      (p[1] != ':' && p[1] != '[' && p[1] != '-')); p++)
 		if (*p == '\\' && p[1])
-		    p++;
+		    chuck(p);
 
 	    /* The character after the option name specifies the type. */
 	    c = *p;
@@ -1455,7 +1461,7 @@ parse_cadef(char *nam, char **args)
 		    xor[0] = xor[1] = NULL;
 		}
                 zsfree(xor[xnum]);
-		xor[xnum] = ztrdup(rembslashcolon(name));
+		xor[xnum] = ztrdup(name);
 	    }
 	    if (c == ':') {
 		/* There's at least one argument. */
@@ -1527,7 +1533,7 @@ parse_cadef(char *nam, char **args)
 
 	    opt->next = NULL;
 	    opt->gsname = doset;
-	    opt->name = ztrdup(rembslashcolon(name));
+	    opt->name = ztrdup(name);
 	    if (descr)
 		opt->descr = ztrdup(descr);
 	    else if (adpre && oargs && !oargs->next) {
diff --git a/Test/Y03arguments.ztst b/Test/Y03arguments.ztst
index fa4589374..4761e47a3 100644
--- a/Test/Y03arguments.ztst
+++ b/Test/Y03arguments.ztst
@@ -78,11 +78,28 @@
 >line: {tst rest arg2 }{}
 >line: {tst rest arg2 rest }{}
 
- tst_arguments '-\+[opt]'
+ # We test -+ and -= here as they are documented to work when escaped. Other
+ # options containing special characters, like -: and -\, are currently
+ # problematic for various reasons (though they would probably pass this basic
+ # check)
+ tst_arguments -s : '-\+[opt1]' '-\=[opt2]' '-a[opt3]' '-b[opt4]'
  comptest $'tst -\C-d'
-0:-+
->DESCRIPTION:{option}
->NO:{-+  -- opt}
+ comptest $'tst -+\C-d'
+ comptest $'tst -=\C-d'
+0:slash-escaped option name (-+, -=)
+>DESCRIPTION:{option}
+>NO:{-+  -- opt1}
+>NO:{-=  -- opt2}
+>NO:{-a  -- opt3}
+>NO:{-b  -- opt4}
+>DESCRIPTION:{option}
+>NO:{-=  -- opt2}
+>NO:{-a  -- opt3}
+>NO:{-b  -- opt4}
+>DESCRIPTION:{option}
+>NO:{-+  -- opt1}
+>NO:{-a  -- opt3}
+>NO:{-b  -- opt4}
 
  tst_arguments -+o
  comptest $'tst -\t\t\t\C-w\C-w+\t\t\t'