* [PATCH] computil: Fix inconsistent handling of slash-escaped option names
@ 2018-12-21 9:41 dana
2018-12-31 16:25 ` dana
0 siblings, 1 reply; 2+ messages in thread
From: dana @ 2018-12-21 9:41 UTC (permalink / raw)
To: Zsh workers
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'
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] computil: Fix inconsistent handling of slash-escaped option names
2018-12-21 9:41 [PATCH] computil: Fix inconsistent handling of slash-escaped option names dana
@ 2018-12-31 16:25 ` dana
0 siblings, 0 replies; 2+ messages in thread
From: dana @ 2018-12-31 16:25 UTC (permalink / raw)
To: Zsh workers
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'
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-12-31 16:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-21 9:41 [PATCH] computil: Fix inconsistent handling of slash-escaped option names dana
2018-12-31 16:25 ` dana
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
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).