* Re: [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p [not found] ` <20230914105954.20BD1C051D0@vcs2.savannah.gnu.org> @ 2023-09-14 12:25 ` Philip Kaludercic 2023-09-14 12:59 ` Eli Zaretskii 2023-09-14 18:49 ` Thierry Volpiatto 0 siblings, 2 replies; 6+ messages in thread From: Philip Kaludercic @ 2023-09-14 12:25 UTC (permalink / raw) To: emacs-devel; +Cc: Thierry Volpiatto ELPA Syncer <elpasync@gnu.org> writes: > branch: elpa/helm > commit 07dacfe2e2db980a9e42afef2fc8539c155fdd0d > Author: Thierry Volpiatto <thievol@posteo.net> > Commit: Thierry Volpiatto <thievol@posteo.net> > > Prefer string-match-p over string-suffix-p > --- > helm-lib.el | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/helm-lib.el b/helm-lib.el > index 2a93271f57..70df22089c 100644 > --- a/helm-lib.el > +++ b/helm-lib.el > @@ -1732,7 +1732,7 @@ Directories expansion is not supported." > (with-temp-buffer > (call-process-shell-command > (format cmd > - (if (string-suffix-p ".gz" file) > + (if (string-match-p ".gz\\'" file) Is there a reason for this preference? Also, I assume you want to quote the period in ".gz\\'"? If so, I can recommend using `rx' or `wildcard-to-regexp' to avoid mistakes like these. > "gzip -c -q -d" "cat") > (shell-quote-argument file) > regexp) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p 2023-09-14 12:25 ` [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p Philip Kaludercic @ 2023-09-14 12:59 ` Eli Zaretskii 2023-09-14 13:06 ` Philip Kaludercic 2023-09-14 18:49 ` Thierry Volpiatto 1 sibling, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2023-09-14 12:59 UTC (permalink / raw) To: Philip Kaludercic; +Cc: emacs-devel, thievol > From: Philip Kaludercic <philipk@posteo.net> > Cc: Thierry Volpiatto <thievol@posteo.net> > Date: Thu, 14 Sep 2023 12:25:55 +0000 > > ELPA Syncer <elpasync@gnu.org> writes: > > > --- a/helm-lib.el > > +++ b/helm-lib.el > > @@ -1732,7 +1732,7 @@ Directories expansion is not supported." > > (with-temp-buffer > > (call-process-shell-command > > (format cmd > > - (if (string-suffix-p ".gz" file) > > + (if (string-match-p ".gz\\'" file) > > Is there a reason for this preference? I think this change is simply incorrect: the first argument of string-suffix-p is not interpreted as a regexp, but as a simple literal string (the implementation uses compare-strings internally). > Also, I assume you want to quote the period in ".gz\\'"? No need to quote it, see above. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p 2023-09-14 12:59 ` Eli Zaretskii @ 2023-09-14 13:06 ` Philip Kaludercic 2023-09-14 13:13 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Philip Kaludercic @ 2023-09-14 13:06 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel, thievol Eli Zaretskii <eliz@gnu.org> writes: >> From: Philip Kaludercic <philipk@posteo.net> >> Cc: Thierry Volpiatto <thievol@posteo.net> >> Date: Thu, 14 Sep 2023 12:25:55 +0000 >> >> ELPA Syncer <elpasync@gnu.org> writes: >> >> > --- a/helm-lib.el >> > +++ b/helm-lib.el >> > @@ -1732,7 +1732,7 @@ Directories expansion is not supported." >> > (with-temp-buffer >> > (call-process-shell-command >> > (format cmd >> > - (if (string-suffix-p ".gz" file) >> > + (if (string-match-p ".gz\\'" file) >> >> Is there a reason for this preference? > > I think this change is simply incorrect: the first argument of > string-suffix-p is not interpreted as a regexp, but as a simple > literal string (the implementation uses compare-strings internally). But why is it incorrect? `string-suffix-p' is passed a string, while `string-match-p' takes a regular expression, that might be too liberal but should still match everything the previous check did -- or am I missing something? >> Also, I assume you want to quote the period in ".gz\\'"? > > No need to quote it, see above. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p 2023-09-14 13:06 ` Philip Kaludercic @ 2023-09-14 13:13 ` Eli Zaretskii 2023-09-14 16:21 ` Philip Kaludercic 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2023-09-14 13:13 UTC (permalink / raw) To: Philip Kaludercic; +Cc: emacs-devel, thievol > From: Philip Kaludercic <philipk@posteo.net> > Cc: emacs-devel@gnu.org, thievol@posteo.net > Date: Thu, 14 Sep 2023 13:06:47 +0000 > > Eli Zaretskii <eliz@gnu.org> writes: > > > I think this change is simply incorrect: the first argument of > > string-suffix-p is not interpreted as a regexp, but as a simple > > literal string (the implementation uses compare-strings internally). > > But why is it incorrect? `string-suffix-p' is passed a string, while > `string-match-p' takes a regular expression, that might be too liberal > but should still match everything the previous check did -- or am I > missing something? Why do the change when string-suffix-p already ensures there's nothing in the second argument after the suffix? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p 2023-09-14 13:13 ` Eli Zaretskii @ 2023-09-14 16:21 ` Philip Kaludercic 0 siblings, 0 replies; 6+ messages in thread From: Philip Kaludercic @ 2023-09-14 16:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel, thievol Eli Zaretskii <eliz@gnu.org> writes: >> From: Philip Kaludercic <philipk@posteo.net> >> Cc: emacs-devel@gnu.org, thievol@posteo.net >> Date: Thu, 14 Sep 2023 13:06:47 +0000 >> >> Eli Zaretskii <eliz@gnu.org> writes: >> >> > I think this change is simply incorrect: the first argument of >> > string-suffix-p is not interpreted as a regexp, but as a simple >> > literal string (the implementation uses compare-strings internally). >> >> But why is it incorrect? `string-suffix-p' is passed a string, while >> `string-match-p' takes a regular expression, that might be too liberal >> but should still match everything the previous check did -- or am I >> missing something? > > Why do the change when string-suffix-p already ensures there's nothing > in the second argument after the suffix? I don't know, that is what I wanted to ask Thierry. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p 2023-09-14 12:25 ` [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p Philip Kaludercic 2023-09-14 12:59 ` Eli Zaretskii @ 2023-09-14 18:49 ` Thierry Volpiatto 1 sibling, 0 replies; 6+ messages in thread From: Thierry Volpiatto @ 2023-09-14 18:49 UTC (permalink / raw) To: Philip Kaludercic; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 1501 bytes --] Philip Kaludercic <philipk@posteo.net> writes: > ELPA Syncer <elpasync@gnu.org> writes: > >> branch: elpa/helm >> commit 07dacfe2e2db980a9e42afef2fc8539c155fdd0d >> Author: Thierry Volpiatto <thievol@posteo.net> >> Commit: Thierry Volpiatto <thievol@posteo.net> >> >> Prefer string-match-p over string-suffix-p >> --- >> helm-lib.el | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/helm-lib.el b/helm-lib.el >> index 2a93271f57..70df22089c 100644 >> --- a/helm-lib.el >> +++ b/helm-lib.el >> @@ -1732,7 +1732,7 @@ Directories expansion is not supported." >> (with-temp-buffer >> (call-process-shell-command >> (format cmd >> - (if (string-suffix-p ".gz" file) >> + (if (string-match-p ".gz\\'" file) > > Is there a reason for this preference? Just a preference, I tried string-suffix-p but I prefer string-match-p which is more readable, for me at least. > Also, I assume you want to quote the period in ".gz\\'"? Yes, my mistake. > If so, I can recommend using `rx' or `wildcard-to-regexp' to avoid > mistakes like these. No thanks, I hate rx and unneeded complications more generally, I am used to plain regexps. > >> "gzip -c -q -d" "cat") >> (shell-quote-argument file) >> regexp) -- Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 686 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-14 18:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <169468919249.10884.8856179202519044902@vcs2.savannah.gnu.org>
[not found] ` <20230914105954.20BD1C051D0@vcs2.savannah.gnu.org>
2023-09-14 12:25 ` [nongnu] elpa/helm 07dacfe2e2 08/11: Prefer string-match-p over string-suffix-p Philip Kaludercic
2023-09-14 12:59 ` Eli Zaretskii
2023-09-14 13:06 ` Philip Kaludercic
2023-09-14 13:13 ` Eli Zaretskii
2023-09-14 16:21 ` Philip Kaludercic
2023-09-14 18:49 ` Thierry Volpiatto
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git 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 read-only IMAP folder(s) and NNTP newsgroup(s).