zsh-workers
 help / color / Atom feed
* [PATCH] vared: Escape empty elements of arrays
@ 2020-05-19 23:54 Daniel Shahaf
  2020-05-21  7:05 ` Daniel Shahaf
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Shahaf @ 2020-05-19 23:54 UTC (permalink / raw)
  To: zsh-workers; +Cc: Markus Näher

---
Found this while replying to Markus.

The %prep block is copied verbatim from several other Test/X*.ztst
files.  (Some of them do the zmodload at the end, but some not.  I guess
that doesn't matter.)

Cheers,

Daniel

 Src/Zle/zle_main.c       |  5 ++++-
 Test/X05zlebuiltins.ztst | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 Test/X05zlebuiltins.ztst

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 8c0534708..2f6a53a40 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -1721,7 +1721,8 @@ bin_vared(char *name, char **args, Options ops, UNUSED(int func))
 	    return 1;
 	}
 	if (v->isarr) {
-	    /* Array: check for separators and quote them. */
+	    /* Array: check for separators and empty elements and quote them. */
+	    /* ### TODO: should probably use quotestring() rather than reinvent it */
 	    char **arr = getarrvalue(v), **aptr, **tmparr, **tptr;
 	    tptr = tmparr = (char **)zhalloc(sizeof(char *)*(arrlen(arr)+1));
 	    for (aptr = arr; *aptr; aptr++) {
@@ -1763,6 +1764,8 @@ bin_vared(char *name, char **args, Options ops, UNUSED(int func))
 		    *nptr = '\0';
 		    /* Stick this into the array of words to join up */
 		    *tptr++ = newstr;
+		} else if (t == *aptr) {
+		    *tptr++ = "''";
 		} else
 		    *tptr++ = *aptr; /* No, keep original array element */
 	    }
diff --git a/Test/X05zlebuiltins.ztst b/Test/X05zlebuiltins.ztst
new file mode 100644
index 000000000..1bf7b18b0
--- /dev/null
+++ b/Test/X05zlebuiltins.ztst
@@ -0,0 +1,22 @@
+%prep
+
+  if [[ $OSTYPE = cygwin ]]; then
+    ZTST_unimplemented="the zsh/zpty module does not work on Cygwin"
+  elif ( zmodload zsh/zpty 2>/dev/null ); then
+    . $ZTST_srcdir/comptest
+    comptestinit -z $ZTST_testdir/../Src/zsh
+  else
+    ZTST_unimplemented="the zsh/zpty module is not available"
+  fi
+
+%test
+
+  zpty_run 'a=("foo foo" "" "bar")'
+  zletest $'vared a\n\C-xw'
+0:vared: basic quoting test
+>BUFFER: foo\ foo '' bar
+>CURSOR: 15
+
+%clean
+
+  zmodload -ui zsh/zpty

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

* Re: [PATCH] vared: Escape empty elements of arrays
  2020-05-19 23:54 [PATCH] vared: Escape empty elements of arrays Daniel Shahaf
@ 2020-05-21  7:05 ` Daniel Shahaf
  2020-05-21 10:57   ` Oliver Kiddle
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Shahaf @ 2020-05-21  7:05 UTC (permalink / raw)
  To: zsh-workers

Daniel Shahaf wrote on Tue, 19 May 2020 23:54 +0000:
> +++ b/Test/X05zlebuiltins.ztst
> @@ -0,0 +1,22 @@
> +%test
> +
> +  zpty_run 'a=("foo foo" "" "bar")'
> +  zletest $'vared a\n\C-xw'
> +0:vared: basic quoting test
> +>BUFFER: foo\ foo '' bar
> +>CURSOR: 15  

The patch is wrong, and the test here is incomplete.

First, because of how vared works —

> > If an array or array slice is being edited, separator characters as defined
> > in tt($IFS) will be shown quoted with a backslash, as will backslashes
> > themselves.  Conversely, when the edited text is split into an array, a
> > backslash quotes an immediately following separator character or backslash;
> > no other special handling of backslashes, or any handling of quotes, is
> > performed.

— it seems to be impossible for the result of a vared operation to
include empty elements.  In particular, editing an array with empty
elements in master silently drops them.  With the patch, the elements
would become «''» (two bytes).

The test didn't catch this because it didn't check the variable's value
after the edit.

---

I assume there should be a way to use vared to make an array element
empty.

Since the documentation specifically promises its own quoting
semantics, I guess we have to continue to support them, even if we
might've designed differently if we could redesign it from scratch.

All in all, I guess we should add another a mode where vared would use
normal shell quoting semantics.  For array variables that'd be
well-defined.  For editing scalars there'd be a question of how to handle
unescaped spaces.

I'll fix the test and commit it later, but I don't have the brainwidth
to add support for empty elements if it's not just a quick fix.

Patches welcome.

Cheers,

Daniel

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

* Re: [PATCH] vared: Escape empty elements of arrays
  2020-05-21  7:05 ` Daniel Shahaf
@ 2020-05-21 10:57   ` Oliver Kiddle
  2020-05-21 16:32     ` Daniel Shahaf
  0 siblings, 1 reply; 4+ messages in thread
From: Oliver Kiddle @ 2020-05-21 10:57 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

Daniel Shahaf wrote:
> — it seems to be impossible for the result of a vared operation to
> include empty elements.  In particular, editing an array with empty
> elements in master silently drops them.  With the patch, the elements
> would become «''» (two bytes).

For editing arrays, I typically use the following:

  alias lvared="IFS=\$'\n\n' vared"

That preserves empty elements from blank lines.

Oliver

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

* Re: [PATCH] vared: Escape empty elements of arrays
  2020-05-21 10:57   ` Oliver Kiddle
@ 2020-05-21 16:32     ` Daniel Shahaf
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Shahaf @ 2020-05-21 16:32 UTC (permalink / raw)
  To: zsh-workers

Oliver Kiddle wrote on Thu, 21 May 2020 12:57 +0200:
> Daniel Shahaf wrote:
> > — it seems to be impossible for the result of a vared operation to
> > include empty elements.  In particular, editing an array with empty
> > elements in master silently drops them.  With the patch, the elements
> > would become «''» (two bytes).  
> 
> For editing arrays, I typically use the following:
> 
>   alias lvared="IFS=\$'\n\n' vared"
> 
> That preserves empty elements from blank lines.

Thanks!  I'll see if I can add this to the manual as a tip.  And maybe
emit a warning if an array is edited that has empty elements.

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 23:54 [PATCH] vared: Escape empty elements of arrays Daniel Shahaf
2020-05-21  7:05 ` Daniel Shahaf
2020-05-21 10:57   ` Oliver Kiddle
2020-05-21 16:32     ` Daniel Shahaf

zsh-workers

Archives are clonable: git clone --mirror http://inbox.vuxu.org/zsh-workers

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git