[PATCH] vared: Escape empty elements of arrays

[PATCH] vared: Escape empty elements of arrays

[Daniel Shahaf]

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

Re: [PATCH] vared: Escape empty elements of arrays

[Daniel Shahaf]

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

Re: [PATCH] vared: Escape empty elements of arrays

[Oliver Kiddle]

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

Re: [PATCH] vared: Escape empty elements of arrays

[Daniel Shahaf]

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.