From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: [Bug] S-flag imposes non-greedy match where it shouldn't
Date: Sat, 28 Dec 2019 21:00:17 +0000 [thread overview]
Message-ID: <20191228210017.2cdgwgpqrssrfhgp@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <CAKc7PVDx6WEFcZFS3TQ6+rF1YpA3jkD5sWwr0LCNE_c5MYkxVg@mail.gmail.com>
Sebastian Gniazdowski wrote on Sat, Dec 28, 2019 at 20:04:21 +0100:
> On Fri, 27 Dec 2019 at 06:30, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > Sebastian Gniazdowski wrote on Thu, Dec 26, 2019 at 19:35:05 +0100:
> > > +++ b/Doc/Zsh/expn.yo
> > > @@ -1399,6 +1399,20 @@ from the beginning and with tt(%) start from the end of the string.
> > > With substitution via tt(${)...tt(/)...tt(}) or
> > > tt(${)...tt(//)...tt(}), specifies non-greedy matching, i.e. that the
> > > shortest instead of the longest match should be replaced.
> > > +The substring search means that the pattern is matched skipping the
> > > +parts of the input string starting from the direction set by the use
> > > +of tt(#) or tt(%).
> >
> > I don't understand this sentence. What does "skipping" mean?
>
> It means that parts of the string are being skipped when they don't
> match when moving to the other end. Does the sentence need an update?
Yes. Feel free to also add a paragraph break, and/or to change the incumbent
text, too.
> > > +For example, to match a pattern starting from the
> > > +end, one could use:
> > > +
> > > +example(str="abcXXXdefXXXghi"
> > > +out=${(S)str%%(#b)([^X])X##}
> > > +out=$out${match[1]}
> > > +)
> > > +
> > > +The result is tt(abcXXXdefghi).
> >
> > That's not correct. The output is abcXXXdefXXXghi (in 'zsh -f') or
> > abcXXXdeghif (with extendedglob set), but not abcXXXdefghi.
>
> I've sent an updated patch half hour before your email. It contains
> the correct example.
>
I saw it, but most of my feedback applied to it too.
I think the last sentence of that patch is the most important one, since it's
the only one that actually gives the general rule. I'd put it nearer the top.
> > I doubt this example would clarify the meaning of ${(S)} to people who
> > encounter it for the first time. Please use a more minimal example.
> > Specific issues:
> > - (...) This is documentation, not
> > a homework problem; the answer should be obvious. Something like
> > «out="${out}+${match[1]}"» would address this — but…
>
> I think that many examples in the man pages are like that – they don't
> go the obvious path of just demonstrating the usage but instead, they
> cover some edge case that, after (sometimes quite long) thinking
> reveal something very peculiar about the feature.
So what? We're not going to accept a patch that adds an unclear explanation
simply because other explanations are unclear.
New documentation should be clear. If any of the existing documentation is
unclear, we should fix that, too.
> There are better examples of this, however, the best that I've found
> currently is the one used for the #b glob flag:
>
> foo="a string with a message"
> if [[ $foo = (a|an)' '(#b)(*)' '* ]]; then
> print ${foo[$mbegin[1],$mend[1]]}
> fi
>
> The example prints `string with a', and the user has a "homework" of
> untangling a few points:
> - why it isn't "string with a message" (it's because the final ' '*
> part that requires a space after the final word of the (*) part),
> - why the answer isn't "message" (the same as above plus the fact that
> there's no * before (a|an) and the greediness).
>
> If not the homework-attitude of the examples in the man page, the
> example would have been
>
> if [[ "a string with a message" = (#b)a' '(*) ]]; then
>
> and would give the answer "string with a message". This would have
> been the obvious-demonstration attitude that I've referred to.
You can't actually get rid of the variable $foo; it's needed for the «print»
call on the next line. Otherwise, I agree. I'll go ahead and make the change,
and also change the spaces to underscores. Thanks for pointing this out. Do
you know any other examples that have room for improvement?
> > - … the use of advanced pattern matching features needlessly raises the
> > learning curve.
>
> I can add the mention that the example needs EXTENDED_GLOB. Overall I
> think that the example:
> - is nice because it shows how to make the (S)...%% substitution
> behave as the intuition would suggest,
Let's not lose sight of the wood for the trees. The purpose of the
documentation is first and foremost to describe what a feature _does_, be it
intuitive or not. Describing how to coerce it into doing other things is
secondary. Your (revised) patch puts the cart before the horse: it describes
your "trick" before describing what ${(S)%%} actually does. Please change
that.
If you then want to recommend left-anchoring the pattern in order to force
a match that starts farther from the end to be used, that would be fine.
And if the left-anchoring example requires capturing groups, so be it — but you
could probably give an example that doesn't. (passwd(5) lines come to mind.)
I wonder if there's anything else the documentation could recommend. Your
trick boils down to using captured negated character classes as a poor man's
negative lookbehind assertion, but we have the zsh/pcre module which supports
real lookaround assertions (as well as resetting the start of the
match, \K), so perhaps that could be used? Or perhaps there's a way to get the
"intuitive" behaviour by reversing the string, using ${(S)##}, and reversing it
again.
> - it's the only place in the documentation that uses the (#b) flag
> with #/% substitution, showing that it's possible to use it in that
> place,
We can add a separate example for that under (#b), which is the more advanced
of these topics, and subject the explanation of (S) to KISS.
> - it isn't that complex for someone that knows #b flag and the $match parameter.
The documentation is aimed at everyone, including people who don't already know (#b).
> > > It would have been tt(abcXXXdefXXghif)
> > > +if not the tt([^X]) part, as despite the tt(%%) specifies a greedy
> > > +match, the substring matching works by trying matches from right to
> > > +left and stops at a first valid match.
> >
> > There are some grammatical errors here (e.g., s/(?<=specif)ies/ying/), but
> > let's not worry about them until the rest of the patch isn't a moving target.
>
> I think that grammar is correct here. Did you maybe misread the sentence?
No, I didn't. I was taught that "despite" should always be followed by a noun
phrase, never by a sentence.
Cheers,
Daniel
next prev parent reply other threads:[~2019-12-28 21:01 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-18 20:41 Sebastian Gniazdowski
2019-12-18 20:44 ` Sebastian Gniazdowski
2019-12-19 15:29 ` Daniel Shahaf
2019-12-26 18:35 ` Sebastian Gniazdowski
2019-12-27 4:54 ` Sebastian Gniazdowski
2019-12-27 5:09 ` Sebastian Gniazdowski
2019-12-27 5:29 ` Daniel Shahaf
2019-12-28 19:04 ` Sebastian Gniazdowski
2019-12-28 20:34 ` Bart Schaefer
2019-12-28 21:00 ` Daniel Shahaf [this message]
2019-12-29 0:56 ` Sebastian Gniazdowski
2019-12-29 2:05 ` Daniel Shahaf
2019-12-29 3:14 ` Sebastian Gniazdowski
2019-12-30 18:00 ` [PATCH] zshexpn: Expand documentation of (S) (was: Re: [Bug] S-flag imposes non-greedy match where it shouldn't) Daniel Shahaf
2019-12-30 18:11 ` Roman Perepelitsa
[not found] ` <CAKc7PVAXLpKqZvmbazZK=mvcz8T-AHJXKusut6aEjkkSLzgdbw@mail.gmail.com>
2019-12-30 20:01 ` Roman Perepelitsa
2019-12-30 20:20 ` Sebastian Gniazdowski
2019-12-30 21:24 ` ${(S)%%*} doesn't match the empty string (was: Re: [PATCH] zshexpn: Expand documentation of (S) (was: Re: [Bug] S-flag imposes non-greedy match where it shouldn't)) Daniel Shahaf
2019-12-30 21:44 ` Roman Perepelitsa
2019-12-30 22:11 ` Sebastian Gniazdowski
2019-12-30 22:34 ` Peter Stephenson
2019-12-30 21:40 ` [PATCH] zshexpn: Expand documentation of (S) (was: Re: [Bug] S-flag imposes non-greedy match where it shouldn't) Roman Perepelitsa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191228210017.2cdgwgpqrssrfhgp@tarpaulin.shahaf.local2 \
--to=d.s@daniel.shahaf.name \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).