histsubstpattern in zmv

histsubstpattern in zmv

[Stephane Chazelas]

The csh-style ${var:s/foo/bar} is often more legible and easy
to cumulate than the Korn-style equivalent ${var/foo/bar}

Compare:

${${(L)var//[[:blank:]]/_}//\/foo[^/]#}

with:

${var:l:gs/[[:blank:]]/_:gs|/foo[^/]#|}

The csh-style also has :& to repeat.

But that needs set -o histsubstpattern and in zmv, emulate -LR
zsh resets options.

Would it be possibe to enable it in zmv along with extendedglob
which is already enabled there, or if not allow the user to enable it,
or maybe even better introduced some :+s/pattern/repl/
:-s/string/repl/ or :S/pattern/replacement/ so we can use both
at the same time, like ${f:gs/***/3-stars/:gS/???/any-3-chars} ?

Thanks,
Stephane

Re: histsubstpattern in zmv

[Peter Stephenson]

On 01/06/2023 19:35 Stephane Chazelas <stephane@chazelas.org> wrote:
> The csh-style ${var:s/foo/bar} is often more legible and easy
> to cumulate than the Korn-style equivalent ${var/foo/bar}
>..
> But that needs set -o histsubstpattern and in zmv, emulate -LR
> zsh resets options.
> 
> Would it be possibe to enable it in zmv along with extendedglob
> which is already enabled there, or if not allow the user to enable it,
> or maybe even better introduced some :+s/pattern/repl/
> :-s/string/repl/ or :S/pattern/replacement/ so we can use both
> at the same time, like ${f:gs/***/3-stars/:gS/???/any-3-chars} ?

I can't see any good reason not to enable it.  The range of
likely cases where this would cause problems is rather limited ---
I'd probably instinctively quote pattern characters anyway.  But
if anyone thinks we should consider something more sophisticated
we can.

pws

diff --git a/Doc/Zsh/contrib.yo b/Doc/Zsh/contrib.yo
index 96de5aa9b..60553210c 100644
--- a/Doc/Zsh/contrib.yo
+++ b/Doc/Zsh/contrib.yo
@@ -4618,12 +4618,16 @@ example(zmv '(*).lis' '$1.txt')
 renames `tt(foo.lis)' to `tt(foo.txt)', `tt(my.old.stuff.lis)' to
 `tt(my.old.stuff.txt)', and so on.
 
-The pattern is always treated as an tt(EXTENDED_GLOB) pattern.  Any file
-whose name is not changed by the substitution is simply ignored.  Any
-error (a substitution resulted in an empty string, two substitutions gave
-the same result, the destination was an existing regular file and tt(-f)
-was not given) causes the entire function to abort without doing
-anything.
+The pattern is always treated as an tt(EXTENDED_GLOB) pattern.
+Furthermore, the tt(HIST_SUBST_PATTERN) option is enabled in order
+to allow the use of patterns in history-style substituions, i.e.
+tt(${f:s/)var(pat)tt(/)var(rep)tt(/}).
+
+Any file whose name is not changed by the substitution is simply
+ignored.  Any error (a substitution resulted in an empty string, two
+substitutions gave the same result, the destination was an existing
+regular file and tt(-f) was not given) causes the entire function to
+abort without doing anything.
 
 In addition to pattern replacement, the variable tt($f) can be referred
 to in the second (replacement) argument.  This makes it possible to
diff --git a/Functions/Misc/zmv b/Functions/Misc/zmv
index 269fe5ba5..58b0327cb 100644
--- a/Functions/Misc/zmv
+++ b/Functions/Misc/zmv
@@ -123,7 +123,7 @@
 #   paranoiacs like the author will probably put them there anyway.
 
 emulate -RL zsh
-setopt extendedglob
+setopt extendedglob histsubstpattern
 
 local f g args match mbegin mend files action myname tmpf opt exec
 local opt_f opt_i opt_n opt_q opt_Q opt_s opt_M opt_C opt_L

Re: histsubstpattern in zmv

[Peter Stephenson]

> On 02/06/2023 09:40 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On 01/06/2023 19:35 Stephane Chazelas <stephane@chazelas.org> wrote:
> > The csh-style ${var:s/foo/bar} is often more legible and easy
> > to cumulate than the Korn-style equivalent ${var/foo/bar}
> >..
> > But that needs set -o histsubstpattern and in zmv, emulate -LR
> > zsh resets options.
> > 
> > Would it be possibe to enable it in zmv along with extendedglob
> > which is already enabled there, or if not allow the user to enable it,
> > or maybe even better introduced some :+s/pattern/repl/
> > :-s/string/repl/ or :S/pattern/replacement/ so we can use both
> > at the same time, like ${f:gs/***/3-stars/:gS/???/any-3-chars} ?
> 
> I can't see any good reason not to enable it.

Having said that, adding an :S variant isn't particularly difficult
and is more widely useful.

pws

diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index 6f86d0c54..7bc736470 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -312,7 +312,8 @@ zero) that are neither `tt(.)' nor `tt(/)' and that continue to the end
 of the string.  For example, the extension of
 `tt(foo.orig.c)' is `tt(.c)', and `tt(dir.c/foo)' has no extension.
 )
-item(tt(s/)var(l)tt(/)var(r)[tt(/)])(
+xitem(tt(s/)var(l)tt(/)var(r)[tt(/)])
+item(tt(S/)var(l)tt(/)var(r)[tt(/)])(
 Substitute var(r) for var(l) as described below.
 The substitution is done only for the
 first string that matches var(l).  For arrays and for filename
@@ -324,13 +325,17 @@ perform global substitution, i.e. substitute every occurrence of var(r)
 for var(l).  Note that the tt(g) or tt(:G) must appear in exactly the
 position shown.
 
+The use of tt(S) instead of tt(s) is identical except that
+the source is treated as a pattern, just as if the option
+tt(HIST_SUBST_PATTERN) were set.
+
 See further notes on this form of substitution below.
 )
 item(tt(&))(
-Repeat the previous tt(s) substitution.  Like tt(s), may be preceded
-immediately by a tt(g).  In parameter expansion the tt(&) must appear
-inside braces, and in filename generation it must be quoted with a
-backslash.
+Repeat the previous tt(s) or tt(S) substitution, whichever was most
+recent.  Like tt(s) and tt(S), may be preceded immediately by a tt(g).
+In parameter expansion the tt(&) must appear inside braces, and in
+filename generation it must be quoted with a backslash.
 )
 item(tt(t) [ var(digits) ])(
 Remove all leading pathname components, leaving the final component (tail).
@@ -377,7 +382,8 @@ substitutions or expansions are performed once at the time the qualifier
 is parsed, even before the `tt(:s)' expression itself is divided into
 var(l) and var(r) sides.
 
-If the option tt(HIST_SUBST_PATTERN) is set, var(l) is treated as
+If the option tt(HIST_SUBST_PATTERN) is set or the original substitution
+was started with a capital tt(S), var(l) is treated as
 a pattern of the usual form described in
 ifzman(the section FILENAME GENERATION below)\
 ifnzman(noderef(Filename Generation)).  This can be used in
diff --git a/Src/hist.c b/Src/hist.c
index 7e6394406..b4dc53d90 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -163,6 +163,11 @@ char *hsubl;
 /**/
 char *hsubr;
  
+/* state of histsubstpattern at last substitution */
+
+/**/
+int hsubpatopt;
+
 /* pointer into the history line */
  
 /**/
@@ -624,7 +629,7 @@ histsubchar(int c)
 	    return substfailed();
 	if (!hsubl)
 	    return -1;
-	if (subst(&sline, hsubl, hsubr, gbal))
+	if (subst(&sline, hsubl, hsubr, gbal, 0))
 	    return substfailed();
     } else {
 	/* Line doesn't begin ^foo^bar */
@@ -831,7 +836,7 @@ histsubchar(int c)
 	    if ((c = ingetc()) == 'g') {
 		gbal = 1;
 		c = ingetc();
-		if (c != 's' && c != '&') {
+		if (c != 's' && c != 'S' && c != '&') {
 		    zerr("'s' or '&' modifier expected after 'g'");
 		    return -1;
 		}
@@ -891,11 +896,13 @@ histsubchar(int c)
 		}
 		break;
 	    case 's':
+	    case 'S':
+		hsubpatopt = (c == 'S');
 		if (getsubsargs(sline, &gbal, &cflag))
 		    return -1; /* fall through */
 	    case '&':
 		if (hsubl && hsubr) {
-		    if (subst(&sline, hsubl, hsubr, gbal))
+		    if (subst(&sline, hsubl, hsubr, gbal, hsubpatopt))
 			return substfailed();
 		} else {
 		    herrflush();
@@ -2315,7 +2322,7 @@ casemodify(char *str, int how)
 
 /**/
 int
-subst(char **strptr, char *in, char *out, int gbal)
+subst(char **strptr, char *in, char *out, int gbal, int forcepat)
 {
     char *str = *strptr, *substcut, *sptr;
     int off, inlen, outlen;
@@ -2323,7 +2330,7 @@ subst(char **strptr, char *in, char *out, int gbal)
     if (!*in)
 	in = str, gbal = 0;
 
-    if (isset(HISTSUBSTPATTERN)) {
+    if (isset(HISTSUBSTPATTERN) || forcepat) {
 	int fl = SUB_LONG|SUB_REST|SUB_RETFAIL;
 	char *oldin = in;
 	if (gbal)
diff --git a/Src/subst.c b/Src/subst.c
index 974d6171e..14947ae36 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -4351,6 +4351,8 @@ modify(char **str, char **ptr, int inbrace)
 		break;
 
 	    case 's':
+	    case 'S':
+		hsubpatopt = (**ptr == 'S');
 		c = **ptr;
 		(*ptr)++;
 		ptr1 = *ptr;
@@ -4445,7 +4447,7 @@ modify(char **str, char **ptr, int inbrace)
 		break;
 
 	    case '&':
-		c = 's';
+		c = hsubpatopt ? 'S' : 's';
 		break;
 
 	    case 'g':
@@ -4534,8 +4536,10 @@ modify(char **str, char **ptr, int inbrace)
 			copy = casemodify(tt, CASMOD_UPPER);
 			break;
 		    case 's':
+		    case 'S':
+			hsubpatopt = (c == 'S');
 			if (hsubl && hsubr)
-			    subst(&copy, hsubl, hsubr, gbal);
+			    subst(&copy, hsubl, hsubr, gbal, hsubpatopt);
 			break;
 		    case 'q':
 			copy = quotestring(copy, QT_BACKSLASH_SHOWNULL);
@@ -4620,8 +4624,10 @@ modify(char **str, char **ptr, int inbrace)
 		    *str = casemodify(*str, CASMOD_UPPER);
 		    break;
 		case 's':
+		case 'S':
+		    hsubpatopt = (c == 'S');
 		    if (hsubl && hsubr)
-			subst(str, hsubl, hsubr, gbal);
+			subst(str, hsubl, hsubr, gbal, hsubpatopt);
 		    break;
 		case 'q':
 		    *str = quotestring(*str, QT_BACKSLASH);
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 7990c2958..2fd2f975f 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -2754,3 +2754,13 @@ F:behavior, see http://austingroupbugs.net/view.php?id=888
 0:(users/28784 inspired this) substituting a single-quoted backslash, part #3: control
 >xfooy
 
+ spacestring="string with spaces"
+ print ${spacestring:gs/[[:space:]]/ /}
+ print ${spacestring:g&}
+ print ${spacestring:gS/[[:space:]]//}
+ print ${spacestring:g&}
+0:Different behaviour of :s and :S modifiers
+>string with spaces
+>string with spaces
+>stringwithspaces
+>stringwithspaces

Re: histsubstpattern in zmv

[Stephane Chazelas]

2023-06-02 11:18:38 +0100, Peter Stephenson:
[...]
> > I can't see any good reason not to enable it.
> 
> Having said that, adding an :S variant isn't particularly difficult
> and is more widely useful.
[...]

Many thanks for that.

I take it that supersedes the other patch and that
histsubstpattern will not be enabled in zmv?

I suppose that any time a new history modifier is introduced,
there's a chance that it may break code that did
$var:that-modifier before. Like echo $hard:SOFT here.

grep -rP '\$\w+(\[[^]]*\])?:S' .

in the source of zsh or oh-my-zsh didn't turn up anything,

grep -rP '\$\w+(\[[^]]*\])?:(?![aAceghlpPqQrstux])\w' .

For occurrences of $var:x or $var[...]:x where x is a word
character that is not one of the currently defined modifiers
turns up a few such as:

./Completion/Zsh/Command/_cd:        alt+=( "path-directories-$elem:directory in $tmpcdpath[$elem]:_path_files -W 'tmpcdpath[$elem]' -/" )
./Completion/Unix/Command/_subversion:        ${args/(#b)(*--file*):arg:/$match[1]:file:_files}

So if we were to add a "f" or "d" or "_" modifier today, it
would break some zsh code supplied with zsh.

May be worth applying the patch below today for future-proofing
and a note in the manual that it's unwise to leave a parameter
expansion un-braced if it's followed by a ":"?

diff --git a/Completion/BSD/Command/_rcctl b/Completion/BSD/Command/_rcctl
index 98e4f9846..457c3eb85 100644
--- a/Completion/BSD/Command/_rcctl
+++ b/Completion/BSD/Command/_rcctl
@@ -46,6 +46,6 @@ case $service in
       '*:argument:'
     ;;
   ${(~j:|:)actions}|disable|enable)
-    _arguments "*:service to $words[2]:_services"
+    _arguments "*:service to ${words[2]}:_services"
     ;;
 esac
diff --git a/Completion/Unix/Command/_ant b/Completion/Unix/Command/_ant
index 7401c7449..36c7c0e89 100644
--- a/Completion/Unix/Command/_ant
+++ b/Completion/Unix/Command/_ant
@@ -79,8 +79,8 @@ case $state in
     compset -P '*:'
     compset -S ':*'
     _alternative \
-      "classpath:$state:_path_files -qS: -g '*.(jar|zip)(-.)'" \
-      "classpath:$state:_path_files -r': ' -/" && ret=0
+      "classpath:${state}:_path_files -qS: -g '*.(jar|zip)(-.)'" \
+      "classpath:${state}:_path_files -r': ' -/" && ret=0
   ;;
   property)
     if compset -P 1 '*='; then
diff --git a/Completion/Unix/Command/_ffmpeg b/Completion/Unix/Command/_ffmpeg
index 1329939cd..e5afdac4f 100644
--- a/Completion/Unix/Command/_ffmpeg
+++ b/Completion/Unix/Command/_ffmpeg
@@ -125,7 +125,7 @@ local -a _ffmpeg_argspecs
                 lastopt_description=${lastopt_description//:/\\:}
                 if [[ $example == filename ]]; then
                     lastopt_takesargs=0
-                    lastopt+=":$lastopt_description:_files"
+                    lastopt+=":${lastopt_description}:_files"
                 elif [[ $lastopt == -[asv]pre ]]; then
                     lastopt_takesargs=0
                     lastopt="*$lastopt"
diff --git a/Completion/Unix/Command/_java b/Completion/Unix/Command/_java
index ff6e82645..b2352df7b 100644
--- a/Completion/Unix/Command/_java
+++ b/Completion/Unix/Command/_java
@@ -438,8 +438,8 @@ classpath|sourcepath|bootstrapclasspath|docletpath)
   compset -P '*:'
   compset -S ':*'
   _alternative \
-    "classpath:$state:_path_files -qS: -g '*.(jar|zip)(-.)'" \
-    "classpath:$state:_path_files -r': ' -/" && return
+    "classpath:${state}:_path_files -qS: -g '*.(jar|zip)(-.)'" \
+    "classpath:${state}:_path_files -r': ' -/" && return
   ;;
 
 extdirs)
diff --git a/Completion/Unix/Command/_subversion b/Completion/Unix/Command/_subversion
index 9a0328dca..ccaf310fa 100644
--- a/Completion/Unix/Command/_subversion
+++ b/Completion/Unix/Command/_subversion
@@ -139,7 +139,7 @@ _svn () {
           ;;
           (commit)
             args=(
-	      ${args/(#b)(*--file*):arg:/$match[1]:file:_files}
+	      ${args/(#b)(*--file*):arg:/${match[1]}:file:_files}
               '*:file: _svn_modified "committable"'
             )
           ;;
@@ -189,7 +189,7 @@ _svn () {
 	    args=(
 	    ':propname:(svn:ignore svn:keywords svn:executable svn:eol-style svn:mime-type svn:externals svn:needs-lock svn:global-ignores svn:auto-props)'
 	    ':propval:->propset_propval'
-	    ${args/(#b)(*--file*):arg:/$match[1]:file:_files}
+	    ${args/(#b)(*--file*):arg:/${match[1]}:file:_files}
 	    '*:path or url: _alternative "files:file:_files" "urls:URL:_svn_urls"'
 	    )
 	  ;;
diff --git a/Completion/Zsh/Command/_cd b/Completion/Zsh/Command/_cd
index 961d121e0..9ca846c8f 100644
--- a/Completion/Zsh/Command/_cd
+++ b/Completion/Zsh/Command/_cd
@@ -73,7 +73,7 @@ else
     if zstyle -t ":completion:${curcontext}:path-directories" separate-sections; then
       local elem
       for ((elem=1; elem <= $#tmpcdpath; elem++)); do
-        alt+=( "path-directories-$elem:directory in $tmpcdpath[$elem]:_path_files -W 'tmpcdpath[$elem]' -/" )
+        alt+=( "path-directories-${elem}:directory in ${tmpcdpath[$elem]}:_path_files -W 'tmpcdpath[$elem]' -/" )
       done
     else
       (( $#tmpcdpath )) &&

-- 
Stephane

Re: histsubstpattern in zmv

[Stephane Chazelas]

2023-06-02 16:09:53 +0100, Stephane Chazelas:
[...]
> May be worth applying the patch below today for future-proofing


$ grep -rnP '\$[~^=#]?([$!#?*@]|\w+)(\[[^]]*\])?:(?![aAceghlpPqQrstux])\w' Completion
Completion/Solaris/Command/_ipadm:139:  proto_all_properties=( $^proto_all_properties:value:_ipadm_get_possible_values )

is more complete (accounts for the $# $!... special parameters
and $~var, $^var...) and finds that extra one.

I likely still forget some more.

diff --git a/Completion/Solaris/Command/_ipadm b/Completion/Solaris/Command/_ipadm
index 34d1eacba..c59fc8108 100644
--- a/Completion/Solaris/Command/_ipadm
+++ b/Completion/Solaris/Command/_ipadm
@@ -136,7 +136,7 @@ _ipadm() {
 	proto_icmp_properties=( "max_buf" "recv_buf" "send_buf" )
 	proto_all_properties=( $proto_ipv4_properties $proto_ipv6_properties $proto_tcp_properties
 		$proto_udp_properties $proto_sctp_properties $proto_icmp_properties )
-	proto_all_properties=( $^proto_all_properties:value:_ipadm_get_possible_values )
+	proto_all_properties=( ${^proto_all_properties}:value:_ipadm_get_possible_values )
 
 	proto_propproperties=( "proto" "property" "perm" "current" "persistent" "default" "possible" )
 

> and a note in the manual that it's unwise to leave a parameter
> expansion un-braced if it's followed by a ":"?
[...]

unless the ksharrays option is enabled.

-- 
Stephane

Re: histsubstpattern in zmv

[Peter Stephenson]

On Fri, 2023-06-02 at 16:09 +0100, Stephane Chazelas wrote:
> 2023-06-02 11:18:38 +0100, Peter Stephenson:
> [...]
> > > I can't see any good reason not to enable it.
> > 
> > Having said that, adding an :S variant isn't particularly difficult
> > and is more widely useful.
> [...]
> 
> Many thanks for that.
> 
> I take it that supersedes the other patch and that
> histsubstpattern will not be enabled in zmv?

Yes, indeed.  I'll commit it if there are not further comments (and your
patch if no one has got round to it by then).

pws

Re: histsubstpattern in zmv

[Jun T]

> 2023/06/03 0:09, Stephane Chazelas <stephane@chazelas.org> wrote:
> 
> May be worth applying the patch below today for future-proofing

The commit 0577daf does not include the patch for _rcctl,
and includes Completion/Unix/Command/_ant.rej (garbage).

I've pushed a commit that adds the former and removes the latter.