[PATCH] Re: RFC: region_highlight inter-plugin interoperability: echo service?

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Daniel Shahaf wrote on Sat, 06 Jun 2020 07:20 +0000:
> Thus, we propose to add a memo=* feature to region_highlight, as
> outlined in the first bullet above, based on the patch that is PR57.

Here's a patch.  I've verified that, in conjunction with a z-sy-h patch
posted to [1], it fixes a plugin interoperability issue [4] reported
against Oliver's viexchange plugin.

[[[
diff --git a/Doc/Zsh/zle.yo b/Doc/Zsh/zle.yo
index c928b8ca2..909abeb49 100644
--- a/Doc/Zsh/zle.yo
+++ b/Doc/Zsh/zle.yo
@@ -974,27 +974,39 @@ of the non-editable parts of the command line in tt(PREDISPLAY)
 and tt(POSTDISPLAY) are possible, but note that the tt(P) flag
 is needed for character indexing to include tt(PREDISPLAY).
 
-Each string consists of the following parts:
+Each string consists of the following whitespace-separated parts:
 
 startitemize()
 itemiz(Optionally, a `tt(P)' to signify that the start and end offset that
 follow include any string set by the tt(PREDISPLAY) special parameter;
 this is needed if the predisplay string itself is to be highlighted.
-Whitespace may follow the `tt(P)'.)
-itemiz(A start offset in the same units as tt(CURSOR), terminated by
-whitespace.)
-itemiz(An end offset in the same units as tt(CURSOR), terminated by
-whitespace.)
+Whitespace between the `tt(P)' and the start offset is optional.)
+itemiz(A start offset in the same units as tt(CURSOR).)
+itemiz(An end offset in the same units as tt(CURSOR).)
 itemiz(A highlight specification in the same format as
 used for contexts in the parameter tt(zle_highlight), see
 ifnzman(noderef(Character Highlighting))\
 ifzman(the section `Character Highlighting' below);
-for example, tt(standout) or tt(fg=red,bold)).
+for example, tt(standout) or tt(fg=red,bold).)
+itemiz(Optionally, a string of the form `tt(memo=)var(token)'.
+The var(token) consists of everything between the `tt(=)' and the next
+whitespace, comma, NUL, or the end of the string.
+The var(token) is preserved verbatim but not parsed in any way.
+
+Plugins may use this to identify array elements they have added: for example,
+a plugin might set var(token) to its (the plugin's) name and then use
+`tt(region_highlight=+LPAR() ${region_highlight:#*memo=)var(token)tt(} +RPAR())'
+in order to remove array elements it have added.
+
+(This example uses the `tt(${)var(name)tt(:#)var(pattern)tt(})' array-grepping
+syntax described in
+ifzman(the section `Parameter Expansion' in zmanref(zshexpn))\
+ifnzman(noderef(Parameter Expansion)).))
 enditemize()
 
 For example, 
 
-example(region_highlight=("P0 20 bold"))
+example(region_highlight=("P0 20 bold memo=foobar"))
 
 specifies that the first twenty characters of the text including
 any predisplay string should be highlighted in bold.
@@ -1002,6 +1014,14 @@ any predisplay string should be highlighted in bold.
 Note that the effect of tt(region_highlight) is not saved and disappears
 as soon as the line is accepted.
 
+Note that zsh 5.8 and older do not support the `tt(memo=)var(token)' field
+and may misparse the third (highlight specification) field when a memo
+is given.
+COMMENT(The syntax `tt(0 20 bold, memo=foobar)' (with an auxiliary comma)
+happens to work on both zsh <=5.8 and zsh 5.9-to-be, but that seems to be more of
+an accident of implementation than something we should make a first-class-citizen
+API promise.  It's mentioned in the "Incompatibilities" section of README.)
+
 The final highlighting on the command line depends on both tt(region_highlight)
 and tt(zle_highlight); see
 ifzman(the section CHARACTER HIGHLIGHTING below)\
diff --git a/README b/README
index 8ae615153..9b1b1605f 100644
--- a/README
+++ b/README
@@ -83,6 +83,15 @@ affects you, make the implied colons in the first pattern explicit, as in:
     zstyle ':foo:*:baz:*' style value2
 This will use value1 in both 5.8 and 5.9.
 
+Elements of the region_highlight array have gained a fourth space-separated
+field.  Code written against 5.9 that sets the new field may break under 5.8:
+for example, the element "0 20 bold memo=foo", which is valid under 5.9, would
+not work under 5.8.  (Under the hood, 5.8 does not recognize the space as
+terminating the highlighting specification.)  On the other hand, code that does
+not set the new, fourth field will continue to work under both 5.8 and 5.9.
+(As it happens, adding a comma after "bold" will make both 5.8 and 5.9 do the
+right thing, but this should be viewed as an unsupported hack.)
+
 Incompatibilities between 5.7.1 and 5.8
 ---------------------------------------
 
diff --git a/Src/Zle/zle.h b/Src/Zle/zle.h
index 609493f8c..391586c4a 100644
--- a/Src/Zle/zle.h
+++ b/Src/Zle/zle.h
@@ -447,6 +447,10 @@ struct region_highlight {
      * Any of the flags defined above.
      */
     int flags;
+    /*
+     * User-settable "memo" key.  Metafied.
+     */
+    const char *memo;
 };
 
 /*
diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index 7b8593dec..d9d9503e2 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -212,9 +212,9 @@ static zattr default_atr_on, special_atr_on;
 
 /*
  * Array of region highlights, no special termination.
- * The first element (0) always describes the region between
- * point and mark.  Any other elements are set by the user
- * via the parameter region_highlight.
+ * The first N_SPECIAL_HIGHLIGHTS elements describe special uses of
+ * highlighting, documented under N_SPECIAL_HIGHLIGHTS.
+ * Any other elements are set by the user via the parameter region_highlight.
  */
 
 /**/
@@ -414,16 +414,19 @@ get_region_highlight(UNUSED(Param pm))
 	 arrsize--;
 	 rhp++, arrp++) {
 	char digbuf1[DIGBUFSIZE], digbuf2[DIGBUFSIZE];
-	int atrlen = 0, alloclen;
+	int atrlen, alloclen;
+	const char memo_equals[] = "memo=";
 
 	sprintf(digbuf1, "%d", rhp->start);
 	sprintf(digbuf2, "%d", rhp->end);
 
 	atrlen = output_highlight(rhp->atr, NULL);
 	alloclen = atrlen + strlen(digbuf1) + strlen(digbuf2) +
-	    3; /* 2 spaces, 1 0 */
+	    3; /* 2 spaces, 1 terminating NUL */
 	if (rhp->flags & ZRH_PREDISPLAY)
 	    alloclen += 2; /* "P " */
+	if (rhp->memo)
+	    alloclen += 1 /* space */ + strlen(memo_equals) + strlen(rhp->memo);
 	*arrp = (char *)zhalloc(alloclen * sizeof(char));
 	/*
 	 * On input we allow a space after the flags.
@@ -436,6 +439,12 @@ get_region_highlight(UNUSED(Param pm))
 		(rhp->flags & ZRH_PREDISPLAY) ? "P" : "",
 		digbuf1, digbuf2);
 	(void)output_highlight(rhp->atr, *arrp + strlen(*arrp));
+
+	if (rhp->memo) {
+	    strcat(*arrp, " ");
+	    strcat(*arrp, memo_equals);
+	    strcat(*arrp, rhp->memo);
+	}
     }
     *arrp = NULL;
     return retarr;
@@ -460,6 +469,8 @@ set_region_highlight(UNUSED(Param pm), char **aval)
 	/* no null termination, but include special highlighting at start */
 	int newsize = len + N_SPECIAL_HIGHLIGHTS;
 	int diffsize = newsize - n_region_highlights;
+
+	free_region_highlights_memos();
 	region_highlights = (struct region_highlight *)
 	    zrealloc(region_highlights,
 		     sizeof(struct region_highlight) * newsize);
@@ -476,6 +487,7 @@ set_region_highlight(UNUSED(Param pm), char **aval)
 	 *aval;
 	 rhp++, aval++) {
 	char *strp, *oldstrp;
+	const char memo_equals[] = "memo=";
 
 	oldstrp = *aval;
 	if (*oldstrp == 'P') {
@@ -502,7 +514,44 @@ set_region_highlight(UNUSED(Param pm), char **aval)
 	while (inblank(*strp))
 	    strp++;
 
-	match_highlight(strp, &rhp->atr);
+	strp = (char*) match_highlight(strp, &rhp->atr);
+
+	while (inblank(*strp))
+	    strp++;
+
+	if (strpfx(memo_equals, strp)) {
+	    const char *memo_start = strp + strlen(memo_equals);
+	    const char *i, *memo_end;
+
+	    /* 
+	     * Forward compatibility: end parsing at a comma or whitespace to
+	     * allow the following extensions:
+	     *
+	     * - A fifth field: "0 20 bold memo=foo bar".
+	     *
+	     * - Additional attributes in the fourth field: "0 20 bold memo=foo,bar"
+	     *   and "0 20 bold memo=foo\0bar".
+	     *
+	     * For similar reasons, we don't flag an error if the fourth field
+	     * doesn't start with "memo=" as we expect.
+	     */
+	    i = memo_start;
+
+	    /* ### TODO: Consider optimizing the common case that memo_start to
+	     *           end-of-string is entirely ASCII */
+	    while (1) {
+		int nbytes;
+		convchar_t c = unmeta_one(i, &nbytes);
+
+		if (c == '\0' || c == ',' || inblank(c)) {
+		    memo_end = i;
+		    break;
+		} else
+		    i += nbytes;
+	    }
+	    rhp->memo = ztrduppfx(memo_start, memo_end - memo_start);
+	} else
+	    rhp->memo = NULL;
     }
 
     freearray(av);
@@ -2797,6 +2846,7 @@ zle_refresh_finish(void)
 
     if (region_highlights)
     {
+	free_region_highlights_memos();
 	zfree(region_highlights,
 	      sizeof(struct region_highlight) * n_region_highlights);
 	region_highlights = NULL;
diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index 2b306fdcd..ea18aafde 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -557,6 +557,22 @@ zlegetline(int *ll, int *cs)
 }
 
 
+/*
+ * free() the 'memo' elements of region_highlights.
+ */
+
+/**/
+void
+free_region_highlights_memos(void)
+{
+    struct region_highlight *rhp;
+    for (rhp = region_highlights;
+	 rhp < region_highlights + n_region_highlights;
+	 rhp++) {
+	zsfree(rhp->memo);
+    }
+}
+
 /* Forward reference */
 struct zle_region;
 
@@ -568,6 +584,7 @@ struct zle_region  {
     int start;
     int end;
     int flags;
+    const char *memo;
 };
 
 /* Forward reference */
@@ -632,6 +649,7 @@ zle_save_positions(void)
 	    newrhp->next = NULL;
 	    newrhp->atr = rhp->atr;
 	    newrhp->flags = rhp->flags;
+	    newrhp->memo = ztrdup(rhp->memo);
 	    if (zlemetaline) {
 		newrhp->start = rhp->start_meta;
 		newrhp->end = rhp->end_meta;
@@ -682,6 +700,7 @@ zle_restore_positions(void)
 	     nreg++, oldrhp = oldrhp->next)
 	    ;
 	if (nreg + N_SPECIAL_HIGHLIGHTS != n_region_highlights) {
+	    free_region_highlights_memos();
 	    n_region_highlights = nreg + N_SPECIAL_HIGHLIGHTS;
 	    region_highlights = (struct region_highlight *)
 		zrealloc(region_highlights,
@@ -694,6 +713,7 @@ zle_restore_positions(void)
 
 	    rhp->atr = oldrhp->atr;
 	    rhp->flags = oldrhp->flags;
+	    rhp->memo = oldrhp->memo; /* transferring ownership of the permanently-allocated memory */
 	    if (zlemetaline) {
 		rhp->start_meta = oldrhp->start;
 		rhp->end_meta = oldrhp->end;
@@ -707,6 +727,7 @@ zle_restore_positions(void)
 	    rhp++;
 	}
     } else if (region_highlights) {
+	free_region_highlights_memos();
 	zfree(region_highlights, sizeof(struct region_highlight) *
 	      n_region_highlights);
 	region_highlights  = NULL;
diff --git a/Src/mem.c b/Src/mem.c
index 5951e57ed..a20e3f729 100644
--- a/Src/mem.c
+++ b/Src/mem.c
@@ -1655,7 +1655,7 @@ free(FREE_ARG_T p)
 
 /**/
 mod_export void
-zsfree(char *p)
+zsfree(const char *p)
 {
     if (p)
 	zfree(p, strlen(p) + 1);
diff --git a/Src/prompt.c b/Src/prompt.c
index b65bfb86b..bc9734720 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -1724,10 +1724,11 @@ match_colour(const char **teststrp, int is_fg, int colour)
 /*
  * Match a set of highlights in the given teststr.
  * Set *on_var to reflect the values found.
+ * Return a pointer to the first character not consumed.
  */
 
 /**/
-mod_export void
+mod_export const char *
 match_highlight(const char *teststr, zattr *on_var)
 {
     int found = 1;
@@ -1745,7 +1746,7 @@ match_highlight(const char *teststr, zattr *on_var)
 	    atr = match_colour(&teststr, is_fg, 0);
 	    if (*teststr == ',')
 		teststr++;
-	    else if (*teststr)
+	    else if (*teststr && *teststr != ' ')
 		break;
 	    found = 1;
 	    /* skip out of range colours but keep scanning attributes */
@@ -1758,7 +1759,7 @@ match_highlight(const char *teststr, zattr *on_var)
 
 		    if (*val == ',')
 			val++;
-		    else if (*val)
+		    else if (*val && *val != ' ')
 			break;
 
 		    *on_var |= hl->mask_on;
@@ -1769,6 +1770,8 @@ match_highlight(const char *teststr, zattr *on_var)
 	    }
 	}
     }
+
+    return teststr;
 }
 
 /*
diff --git a/Test/X04zlehighlight.ztst b/Test/X04zlehighlight.ztst
index 475a2e309..7ab050bee 100644
--- a/Test/X04zlehighlight.ztst
+++ b/Test/X04zlehighlight.ztst
@@ -94,6 +94,50 @@
 0:basic region_highlight with 8 colors
 >0m27m24mCDE|32|trueCDE|39|
 
+  zpty_start
+  zpty_input 'rh_widget() { region_highlight+=( "0 4 fg=green memo=someplugin" ); typeset -p region_highlight }'
+  zpty_input 'zle -N rh_widget'
+  zpty_input 'bindkey "\C-a" rh_widget'
+  zpty_enable_zle
+  zpty_input $'\C-a'
+  zpty_line
+  zpty_stop
+0:region_highlight memo information round trips
+>typeset -a region_highlight=( '0 4 fg=green memo=someplugin' )
+
+  zpty_start
+  zpty_input 'rh_widget() { region_highlight+=( "0 4 fg=green memo=someplugin,futureattribute=futurevalue" ); typeset -p region_highlight }'
+  zpty_input 'zle -N rh_widget'
+  zpty_input 'bindkey "\C-a" rh_widget'
+  zpty_enable_zle
+  zpty_input $'\C-a'
+  zpty_line
+  zpty_stop
+0:region_highlight memo information forward compatibility, #1
+>typeset -a region_highlight=( '0 4 fg=green memo=someplugin' )
+
+  zpty_start
+  zpty_input 'rh_widget() { region_highlight+=( "0 4 fg=green memo=someplugin futurefifthfield" ); typeset -p region_highlight }'
+  zpty_input 'zle -N rh_widget'
+  zpty_input 'bindkey "\C-a" rh_widget'
+  zpty_enable_zle
+  zpty_input $'\C-a'
+  zpty_line
+  zpty_stop
+0:region_highlight memo information forward compatibility, #2
+>typeset -a region_highlight=( '0 4 fg=green memo=someplugin' )
+
+  zpty_start
+  zpty_input 'rh_widget() { region_highlight+=( "0 4 fg=green memo=some'$'\0''plugin" ); typeset -p region_highlight }'
+  zpty_input 'zle -N rh_widget'
+  zpty_input 'bindkey "\C-a" rh_widget'
+  zpty_enable_zle
+  zpty_input $'\C-a'
+  zpty_line
+  zpty_stop
+0:region_highlight memo information forward compatibility, #3: NULs
+>typeset -a region_highlight=( '0 4 fg=green memo=some' )
+
   zpty_start
   zpty_input 'rh_widget() { BUFFER="true"; region_highlight+=( "0 4 fg=#040810" ); }'
   zpty_input 'zle -N rh_widget'
]]]

Cheers,

Daniel

> [1] https://github.com/zsh-users/zsh-syntax-highlighting/issues/418
> [2] https://github.com/zsh-users/zsh-syntax-highlighting/issues/579
> [PR57] https://github.com/zsh-users/zsh/pull/57
> [3] https://github.com/zsh-users/zsh/pull/57#discussion_r419548695

[4] https://github.com/okapia/zsh-viexchange/issues/1