From: dana <dana@dana.is>
To: Zsh workers <zsh-workers@zsh.org>
Subject: Re: [PATCH] computil: Fix inconsistent handling of slash-escaped option names
Date: Mon, 31 Dec 2018 10:25:33 -0600 [thread overview]
Message-ID: <111E6C9B-71D3-4558-8E03-E7FF92E655EE@dana.is> (raw)
In-Reply-To: <E023E2BB-4C56-40AF-BDE2-937743F790BF@dana.is>
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'
prev parent reply other threads:[~2018-12-31 16:23 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-21 9:41 dana
2018-12-31 16:25 ` dana [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=111E6C9B-71D3-4558-8E03-E7FF92E655EE@dana.is \
--to=dana@dana.is \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).