zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] internal: quotestring: Drop the 'e' parameter, which no caller uses.
@ 2016-09-09  9:38 Daniel Shahaf
  2016-09-10  5:00 ` Bart Schaefer
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Shahaf @ 2016-09-09  9:38 UTC (permalink / raw)
  To: zsh-workers

---
 Src/Zle/compcore.c   |  2 +-
 Src/Zle/compctl.c    | 20 ++++++++++----------
 Src/Zle/computil.c   |  6 +++---
 Src/Zle/zle_misc.c   |  2 +-
 Src/Zle/zle_tricky.c | 24 ++++++++++++------------
 Src/builtin.c        |  2 +-
 Src/subst.c          | 12 ++++++------
 Src/text.c           |  4 ++--
 Src/utils.c          | 27 ++++-----------------------
 9 files changed, 40 insertions(+), 59 deletions(-)

diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index ae7068f..2f9fb33 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -1055,7 +1055,7 @@ multiquote(char *s, int ign)
 		p += ign;
 	    while (*p) {
 		if (ign >= 0 || p[1])
-		    s = quotestring(s, NULL, *p);
+		    s = quotestring(s, *p);
 		p++;
 	    }
 	}
diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
index ce45762..c2da297 100644
--- a/Src/Zle/compctl.c
+++ b/Src/Zle/compctl.c
@@ -1400,7 +1400,7 @@ printcompctl(char *s, Compctl cc, int printflags, int ispat)
 	    untokenize(p);
 	    quotedzputs(p, stdout);
 	} else
-	    quotedzputs(quotestring(s, NULL, QT_BACKSLASH), stdout);
+	    quotedzputs(quotestring(s, QT_BACKSLASH), stdout);
     }
 
     /* loop through flags w/o args that are set, printing them if so */
@@ -1536,7 +1536,7 @@ printcompctl(char *s, Compctl cc, int printflags, int ispat)
 		char *p = dupstring(s);
 
 		untokenize(p);
-		quotedzputs(quotestring(p, NULL, QT_BACKSLASH), stdout);
+		quotedzputs(quotestring(p, QT_BACKSLASH), stdout);
 	    }
 	}
 	putchar('\n');
@@ -1740,8 +1740,8 @@ static int addwhat;
  * This uses the instring variable exported from zle_tricky.c.
  */
 
-#define quotename(s, e) \
-quotestring(s, e, instring == QT_NONE ? QT_BACKSLASH : instring)
+#define quotename(s) \
+quotestring(s, instring == QT_NONE ? QT_BACKSLASH : instring)
 
 /* Hook functions */
 
@@ -3153,10 +3153,10 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd)
     lpre = zhalloc(lpl + 1);
     memcpy(lpre, s, lpl);
     lpre[lpl] = '\0';
-    qlpre = quotename(lpre, NULL);
+    qlpre = quotename(lpre);
     lsuf = dupstring(s + offs);
     lsl = strlen(lsuf);
-    qlsuf = quotename(lsuf, NULL);
+    qlsuf = quotename(lsuf);
 
     /* First check for ~.../... */
     if (ic == Tilde) {
@@ -3175,11 +3175,11 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd)
     rpre = (*p || *lpre == Tilde || *lpre == Equals) ?
 	(noreal = 0, getreal(tt)) :
 	dupstring(tt);
-    qrpre = quotename(rpre, NULL);
+    qrpre = quotename(rpre);
 
     for (p = lsuf; *p && *p != String && *p != Tick; p++);
     rsuf = *p ? (noreal = 0, getreal(lsuf)) : dupstring(lsuf);
-    qrsuf = quotename(rsuf, NULL);
+    qrsuf = quotename(rsuf);
 
     /* Check if word is a pattern. */
 
@@ -3315,10 +3315,10 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd)
 	/* And get the file prefix. */
 	fpre = dupstring(((s1 == s || s1 == rpre || ic) &&
 			  (*s != '/' || zlemetacs == wb)) ? s1 : s1 + 1);
-	qfpre = quotename(fpre, NULL);
+	qfpre = quotename(fpre);
 	/* And the suffix. */
 	fsuf = dupstrpfx(rsuf, s2 - rsuf);
-	qfsuf = quotename(fsuf, NULL);
+	qfsuf = quotename(fsuf);
 
 	if (comppatmatch && *comppatmatch && (ispattern & 2)) {
 	    int t2;
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 1c90a54..16b681c 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -3554,7 +3554,7 @@ comp_quote(char *str, int prefix)
     if ((x = (prefix && *str == '=')))
 	*str = 'x';
 
-    ret = quotestring(str, NULL, *compqstack);
+    ret = quotestring(str, *compqstack);
 
     if (x)
 	*str = *ret = '=';
@@ -4744,7 +4744,7 @@ cf_ignore(char **names, LinkList ign, char *style, char *path)
     for (; (n = *names); names++) {
 	if (!ztat(n, &nst, 1) && S_ISDIR(nst.st_mode)) {
 	    if (tpwd && nst.st_dev == est.st_dev && nst.st_ino == est.st_ino) {
-		addlinknode(ign, quotestring(n, NULL, QT_BACKSLASH));
+		addlinknode(ign, quotestring(n, QT_BACKSLASH));
 		continue;
 	    }
 	    if (tpar && !strncmp((c = dupstring(n)), path, pl)) {
@@ -4760,7 +4760,7 @@ cf_ignore(char **names, LinkList ign, char *style, char *path)
 		if (found || ((e = strrchr(c, '/')) && e > c + pl &&
 			      !ztat(c, &st, 1) && st.st_dev == nst.st_dev &&
 			      st.st_ino == nst.st_ino))
-		    addlinknode(ign, quotestring(n, NULL, QT_BACKSLASH));
+		    addlinknode(ign, quotestring(n, QT_BACKSLASH));
 	    }
 	}
     }
diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
index a040ca0..fbd40cd 100644
--- a/Src/Zle/zle_misc.c
+++ b/Src/Zle/zle_misc.c
@@ -780,7 +780,7 @@ bracketedpaste(char **args)
 	int n;
 	ZLE_STRING_T wpaste;
 	wpaste = stringaszleline((zmult == 1) ? pbuf :
-	    quotestring(pbuf, NULL, QT_SINGLE_OPTIONAL), 0, &n, NULL, NULL);
+	    quotestring(pbuf, QT_SINGLE_OPTIONAL), 0, &n, NULL, NULL);
 	cuttext(wpaste, n, CUT_REPLACE);
 	if (!(zmod.flags & MOD_VIBUF)) {
 	    kct = -1;
diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index 1d4e1d2..958e79f 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -424,8 +424,8 @@ mod_export int instring, inbackt;
  * This uses the instring variable above.
  */
 
-#define quotename(s, e) \
-quotestring(s, e, instring == QT_NONE ? QT_BACKSLASH : instring)
+#define quotename(s) \
+quotestring(s, instring == QT_NONE ? QT_BACKSLASH : instring)
 
 /* Check if the given string is the name of a parameter and if this *
  * parameter is one worth expanding.                                */
@@ -2004,11 +2004,11 @@ get_comp_string(void)
 
 			new->next = NULL;
 			new->str = dupstrpfx(bbeg, len);
-			new->str = ztrdup(quotename(new->str, NULL));
+			new->str = ztrdup(quotename(new->str));
 			untokenize(new->str);
 			new->pos = begi;
 			*dbeg = '\0';
-			new->qpos = strlen(quotename(predup, NULL));
+			new->qpos = strlen(quotename(predup));
 			*dbeg = '{';
 			i -= len;
 			boffs -= len;
@@ -2067,11 +2067,11 @@ get_comp_string(void)
 			lastbrbeg = new;
 
 			new->str = dupstrpfx(bbeg, len);
-			new->str = ztrdup(quotename(new->str, NULL));
+			new->str = ztrdup(quotename(new->str));
 			untokenize(new->str);
 			new->pos = begi;
 			*dbeg = '\0';
-			new->qpos = strlen(quotename(predup, NULL));
+			new->qpos = strlen(quotename(predup));
 			*dbeg = '{';
 			i -= len;
 			boffs -= len;
@@ -2117,7 +2117,7 @@ get_comp_string(void)
 		    brend = new;
 
 		    new->str = dupstrpfx(bbeg, len);
-		    new->str = ztrdup(quotename(new->str, NULL));
+		    new->str = ztrdup(quotename(new->str));
 		    untokenize(new->str);
 		    new->pos = dp - predup - len + 1;
 		    new->qpos = len;
@@ -2146,11 +2146,11 @@ get_comp_string(void)
 		lastbrbeg = new;
 
 		new->str = dupstrpfx(bbeg, len);
-		new->str = ztrdup(quotename(new->str, NULL));
+		new->str = ztrdup(quotename(new->str));
 		untokenize(new->str);
 		new->pos = begi;
 		*dbeg = '\0';
-		new->qpos = strlen(quotename(predup, NULL));
+		new->qpos = strlen(quotename(predup));
 		*dbeg = '{';
 		boffs -= len;
 		memmove(dbeg, dbeg + len, 1+strlen(dbeg+len));
@@ -2165,7 +2165,7 @@ get_comp_string(void)
 		    p = bp->pos;
 		    l = bp->qpos;
 		    bp->pos = strlen(predup + p + l);
-		    bp->qpos = strlen(quotename(predup + p + l, NULL));
+		    bp->qpos = strlen(quotename(predup + p + l));
 		    memmove(predup + p, predup + p + l, 1+bp->pos);
 		}
 	    }
@@ -2288,7 +2288,7 @@ doexpansion(char *s, int lst, int olst, int explincmd)
     foredel(we - wb, CUT_RAW);
     while ((ss = (char *)ugetnode(vl))) {
 	ret = 0;
-	ss = quotename(ss, NULL);
+	ss = quotename(ss);
 	untokenize(ss);
 	inststr(ss);
 	if (nonempty(vl) || !first) {
@@ -2997,7 +2997,7 @@ processcmd(UNUSED(char **args))
     inststr(" ");
     untokenize(s);
 
-    inststr(quotename(s, NULL));
+    inststr(quotename(s));
 
     zsfree(s);
     done = 1;
diff --git a/Src/builtin.c b/Src/builtin.c
index 3b82c9e..248f929 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -4844,7 +4844,7 @@ bin_print(char *name, char **args, Options ops, int func)
 		break;
 	    case 'q':
 		stringval = curarg ?
-		    quotestring(curarg, NULL, QT_BACKSLASH_SHOWNULL) : &nullstr;
+		    quotestring(curarg, QT_BACKSLASH_SHOWNULL) : &nullstr;
 		*d = 's';
 		print_val(stringval);
 		break;
diff --git a/Src/subst.c b/Src/subst.c
index 4641b4b..1c2027f 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -3615,7 +3615,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 		    char *tmp;
 
 		    for (; *ap; ap++) {
-			tmp = quotestring(*ap, NULL, quotetype);
+			tmp = quotestring(*ap, quotetype);
 			sl = strlen(tmp);
 			*ap = (char *) zhalloc(pre + sl + post + 1);
 			strcpy((*ap) + pre, tmp);
@@ -3628,7 +3628,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 		    }
 		} else
 		    for (; *ap; ap++)
-			*ap = quotestring(*ap, NULL, QT_BACKSLASH_SHOWNULL);
+			*ap = quotestring(*ap, QT_BACKSLASH_SHOWNULL);
 	    } else {
 		int one = noerrs, oef = errflag, haserr = 0;
 
@@ -3658,7 +3658,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 		} else if (quotetype > QT_BACKSLASH) {
 		    int sl;
 		    char *tmp;
-		    tmp = quotestring(val, NULL, quotetype);
+		    tmp = quotestring(val, quotetype);
 		    sl = strlen(tmp);
 		    val = (char *) zhalloc(pre + sl + 2);
 		    strcpy(val + pre, tmp);
@@ -3669,7 +3669,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 		    if (quotetype == QT_DOLLARS)
 		      val[0] = '$';
 		} else
-		    val = quotestring(val, NULL, QT_BACKSLASH_SHOWNULL);
+		    val = quotestring(val, QT_BACKSLASH_SHOWNULL);
 	    } else {
 		int one = noerrs, oef = errflag, haserr;
 
@@ -4274,7 +4274,7 @@ modify(char **str, char **ptr)
 			    subst(&copy, hsubl, hsubr, gbal);
 			break;
 		    case 'q':
-			copy = quotestring(copy, NULL, QT_BACKSLASH_SHOWNULL);
+			copy = quotestring(copy, QT_BACKSLASH_SHOWNULL);
 			break;
 		    case 'Q':
 			{
@@ -4356,7 +4356,7 @@ modify(char **str, char **ptr)
 			subst(str, hsubl, hsubr, gbal);
 		    break;
 		case 'q':
-		    *str = quotestring(*str, NULL, QT_BACKSLASH);
+		    *str = quotestring(*str, QT_BACKSLASH);
 		    break;
 		case 'Q':
 		    {
diff --git a/Src/text.c b/Src/text.c
index cf6d3eb..d387d36 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -1068,11 +1068,11 @@ getredirs(LinkList redirs)
 		     */
 		    if (!has_token(f->name)) {
 			taddchr('\'');
-			taddstr(quotestring(f->name, NULL, QT_SINGLE));
+			taddstr(quotestring(f->name, QT_SINGLE));
 			taddchr('\'');
 		    } else {
 			taddchr('"');
-			taddstr(quotestring(f->name, NULL, QT_DOUBLE));
+			taddstr(quotestring(f->name, QT_DOUBLE));
 			taddchr('"');
 		    }
 		    if (sav)
diff --git a/Src/utils.c b/Src/utils.c
index d209078..b434821 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1047,9 +1047,9 @@ substnamedir(char *s)
     Nameddir d = finddir(s);
 
     if (!d)
-	return quotestring(s, NULL, QT_BACKSLASH);
+	return quotestring(s, QT_BACKSLASH);
     return zhtricat("~", d->node.nam, quotestring(s + strlen(d->dir),
-						  NULL, QT_BACKSLASH));
+						  QT_BACKSLASH));
 }
 
 
@@ -5649,10 +5649,6 @@ addunprintable(char *v, const char *u, const char *uend)
 /*
  * Quote the string s and return the result as a string from the heap.
  *
- * If e is non-zero, the
- * pointer it points to may point to a position in s and in e the position
- * of the corresponding character in the quoted string is returned.
- * 
  * The last argument is a QT_ value defined in zsh.h other than QT_NONE.
  *
  * Most quote styles other than backslash assume the quotes are to
@@ -5665,13 +5661,13 @@ addunprintable(char *v, const char *u, const char *uend)
 
 /**/
 mod_export char *
-quotestring(const char *s, char **e, int instring)
+quotestring(const char *s, int instring)
 {
     const char *u;
     char *v;
     int alloclen;
     char *buf;
-    int sf = 0, shownull = 0;
+    int shownull = 0;
     /*
      * quotesub is used with QT_SINGLE_OPTIONAL.
      * quotesub = 0:  mechanism not active
@@ -5742,10 +5738,6 @@ quotestring(const char *s, char **e, int instring)
 	while (*u) {
 	    uend = u + MB_METACHARLENCONV(u, &cc);
 
-	    if (e && !sf && *e <= u) {
-		*e = v;
-		sf = 1;
-	    }
 	    if (
 #ifdef MULTIBYTE_SUPPORT
 		cc != WEOF &&
@@ -5772,11 +5764,6 @@ quotestring(const char *s, char **e, int instring)
 	}
     } else if (instring == QT_BACKSLASH_PATTERN) {
 	while (*u) {
-	    if (e && !sf && *e == u) {
-		*e = v;
-		sf = 1;
-	    }
-
 	    if (ipattern(*u))
 		*v++ = '\\';
 	    *v++ = *u++;
@@ -5795,8 +5782,6 @@ quotestring(const char *s, char **e, int instring)
 	 */
 	while (*u) {
 	    int dobackslash = 0;
-	    if (e && *e == u)
-		*e = v, sf = 1;
 	    if (*u == Tick || *u == Qtick) {
 		char c = *u++;
 
@@ -5984,10 +5969,6 @@ quotestring(const char *s, char **e, int instring)
 	*v++ = '\'';
     *v = '\0';
 
-    if (e && *e == u)
-	*e = v, sf = 1;
-    DPUTS(e && !sf, "BUG: Wild pointer *e in quotestring()");
-
     v = dupstring(buf);
     zfree(buf, alloclen);
     return v;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] internal: quotestring: Drop the 'e' parameter, which no caller uses.
  2016-09-09  9:38 [PATCH] internal: quotestring: Drop the 'e' parameter, which no caller uses Daniel Shahaf
@ 2016-09-10  5:00 ` Bart Schaefer
  2016-09-11  7:27   ` Daniel Shahaf
  0 siblings, 1 reply; 4+ messages in thread
From: Bart Schaefer @ 2016-09-10  5:00 UTC (permalink / raw)
  To: zsh-workers

On Sep 9,  9:38am, Daniel Shahaf wrote:
} Subject: [PATCH] internal: quotestring: Drop the 'e' parameter, which no c
}
} - * If e is non-zero, the
} - * pointer it points to may point to a position in s and in e the position
} - * of the corresponding character in the quoted string is returned.

Hrm ... this has to have been used at one point e.g. by completion
to keep track of the cursor position while wrapping a substring in
quotes.  I wonder when that stopped being necessary.

Found a use in addmatch() back in zsh-2.4 which persists through 3.0.8
but is gone by 4.3.  Looks like uses started disappearing after 3.1.5
around about workers/5959 in mid-1999.  (The git import from CVS gets
very confused by CVS branches when you go that far back.)

Recall what I've said in a couple of other threads about it being too
hard for completion to intuit what a user means to have remain quoted
and what he expects completion to expand or otherwise treat as unquoted?
The need for this "e" argument seems to have gone away when we decided
to give up and do minimal quote interpretation/insertion.

I don't know what this patch accomplishes other than some dead code
removal, but I guess there's no remaining reason not to commit it.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] internal: quotestring: Drop the 'e' parameter, which no caller uses.
  2016-09-10  5:00 ` Bart Schaefer
@ 2016-09-11  7:27   ` Daniel Shahaf
  2016-09-11 18:10     ` Peter Stephenson
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Shahaf @ 2016-09-11  7:27 UTC (permalink / raw)
  To: zsh-workers

Bart Schaefer wrote on Fri, Sep 09, 2016 at 22:00:03 -0700:
> Recall what I've said in a couple of other threads about it being too
> hard for completion to intuit what a user means to have remain quoted
> and what he expects completion to expand or otherwise treat as unquoted?
> The need for this "e" argument seems to have gone away when we decided
> to give up and do minimal quote interpretation/insertion.

Somewhat related, I don't understand the purpose of tildequote().  I could
document it as follows:

/*
 * tildequote(s, ign): Equivalent to multiquote(s, ign), except that if
 * compqstack[0] == QT_BACKSLASH and s[0] == '~', then that tilde is not
 * quoted.
 */

Which I'm having trouble getting my head around: when would it be a good
idea to decide whether to quote ~ or not based on what _kind_ of quoting
the user happened to use?

Example:

     1	% _f() { compset -q; compadd -f '~/foo foo' }
     2	% compdef _f f
     3	% f \~/foo<TAB>   # becomes ~/foo\\\ foo
     4	% f '~/foo<TAB>   # becomes '~/foo\ foo

Lines 3 and 4 both start with a quoted tilde, but one of them ends with
a quoted tilde and the other with an unquoted tilde.

This seems to be an intentional feature since if tildequote() were
changed to escape the tilde even in the QT_BACKSLASH case, it would be
synonymous with multiquote().

> I don't know what this patch accomplishes other than some dead code
> removal, but I guess there's no remaining reason not to commit it.
> 

Thanks for the inestigation and patch review.

Daniel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] internal: quotestring: Drop the 'e' parameter, which no caller uses.
  2016-09-11  7:27   ` Daniel Shahaf
@ 2016-09-11 18:10     ` Peter Stephenson
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Stephenson @ 2016-09-11 18:10 UTC (permalink / raw)
  To: zsh-workers

On Sun, 11 Sep 2016 07:27:24 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Somewhat related, I don't understand the purpose of tildequote().  I could
> document it as follows:
> 
> /*
>  * tildequote(s, ign): Equivalent to multiquote(s, ign), except that if
>  * compqstack[0] == QT_BACKSLASH and s[0] == '~', then that tilde is not
>  * quoted.
>  */

The basic idea here is that completion leaves ~'s for named directory
expansion alone, even though every other character is supposed to
represent a literal by appropriate quoting, so that to insert a quoted
string from within completion a leading ~ actually needs not to be
quoted.

How this is used I don't remember, so it's perfectly possible, as always
with the completion system, that you're encountering anomalies.  As far
as I know there's no useful case of not quoting a non-leading tilde.

pws


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-09-11 18:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-09  9:38 [PATCH] internal: quotestring: Drop the 'e' parameter, which no caller uses Daniel Shahaf
2016-09-10  5:00 ` Bart Schaefer
2016-09-11  7:27   ` Daniel Shahaf
2016-09-11 18:10     ` Peter Stephenson

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).