unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* [PATCH] Add string-replace-substring to (ice-9 string-fun)
@ 2019-12-20 17:32 lloda
  2020-01-05 11:34 ` Andy Wingo
  2020-01-06  9:08 ` Linus Björnstam
  0 siblings, 2 replies; 8+ messages in thread
From: lloda @ 2019-12-20 17:32 UTC (permalink / raw)
  To: guile-devel

[-- Attachment #1: Type: text/plain, Size: 214 bytes --]


This patch adds string-replace-substring that wingo posted to the mailing list to (ice-9 strings). This is a commonly used function and a good implementation isn't trivial, so I think it deserves inclusion.



[-- Attachment #2: 0001-Add-string-replace-substring-to-ice-9-string-fun.patch --]
[-- Type: application/octet-stream, Size: 1790 bytes --]

From cf5b4a1db8ad4c1060f08f07113791dd34374266 Mon Sep 17 00:00:00 2001
From: Daniel Llorens <lloda@sarc.name>
Date: Fri, 20 Dec 2019 12:53:34 +0100
Subject: [PATCH 1/3] Add string-replace-substring to (ice-9 string-fun)

---
 module/ice-9/string-fun.scm | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/module/ice-9/string-fun.scm b/module/ice-9/string-fun.scm
index c27ff847f..bef841db5 100644
--- a/module/ice-9/string-fun.scm
+++ b/module/ice-9/string-fun.scm
@@ -25,7 +25,8 @@
 	   separate-fields-discarding-char separate-fields-after-char
 	   separate-fields-before-char string-prefix-predicate string-prefix=?
 	   sans-surrounding-whitespace sans-trailing-whitespace
-	   sans-leading-whitespace sans-final-newline has-trailing-newline?))
+	   sans-leading-whitespace sans-final-newline has-trailing-newline?
+           string-replace-substring))
 
 ;;;;
 ;;;
@@ -278,3 +279,25 @@
 ;;;         (fail parts)
 ;;;         (apply return parts))))
 
+
+\f
+;;; {String Fun: string-replace-substring}
+;;;
+
+;; By A. Wingo in https://lists.gnu.org/archive/html/guile-devel/2014-03/msg00058.html
+
+(define (string-replace-substring s substring replacement)
+  "Replace every instance of @var{substring} in string @var{s} by @var{replacement}."
+  (let ((sublen (string-length substring)))
+    (with-output-to-string
+      (lambda ()
+        (let lp ((start 0))
+          (cond
+           ((string-contains s substring start)
+            => (lambda (end)
+                 (display (substring/shared s start end))
+                 (display replacement)
+                 (lp (+ end sublen))))
+           (else
+            (display (substring/shared s start)))))))))
+
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add string-replace-substring to (ice-9 string-fun)
  2019-12-20 17:32 [PATCH] Add string-replace-substring to (ice-9 string-fun) lloda
@ 2020-01-05 11:34 ` Andy Wingo
  2020-01-05 12:12   ` Jan Nieuwenhuizen
  2020-01-06  9:08 ` Linus Björnstam
  1 sibling, 1 reply; 8+ messages in thread
From: Andy Wingo @ 2020-01-05 11:34 UTC (permalink / raw)
  To: lloda; +Cc: guile-devel

On Fri 20 Dec 2019 18:32, lloda <lloda@sarc.name> writes:

> This patch adds string-replace-substring that wingo posted to the
> mailing list to (ice-9 strings). This is a commonly used function and a
> good implementation isn't trivial, so I think it deserves inclusion.

I didn't know that this module existed :)  Sure, why not.  Please fix
the commit message when you push.

Andy



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add string-replace-substring to (ice-9 string-fun)
  2020-01-05 11:34 ` Andy Wingo
@ 2020-01-05 12:12   ` Jan Nieuwenhuizen
  2020-01-05 13:51     ` Christopher Lam
  2020-01-05 14:01     ` lloda
  0 siblings, 2 replies; 8+ messages in thread
From: Jan Nieuwenhuizen @ 2020-01-05 12:12 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

Andy Wingo writes:

> On Fri 20 Dec 2019 18:32, lloda <lloda@sarc.name> writes:
>
>> This patch adds string-replace-substring that wingo posted to the
>> mailing list to (ice-9 strings). This is a commonly used function and a
>> good implementation isn't trivial, so I think it deserves inclusion.
>
> I didn't know that this module existed :)  Sure, why not.  Please fix
> the commit message when you push.

That's great!  This is one of the things that has always amazed and
annoyed me to be missing and I have written several half-baken
implementations of it when I needed something like it.

Would it be too much to ask for some documentation to go with it, so
that we will be able to find it?

Greetings,
janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add string-replace-substring to (ice-9 string-fun)
  2020-01-05 12:12   ` Jan Nieuwenhuizen
@ 2020-01-05 13:51     ` Christopher Lam
  2020-01-05 15:49       ` Arne Babenhauserheide
  2020-01-05 14:01     ` lloda
  1 sibling, 1 reply; 8+ messages in thread
From: Christopher Lam @ 2020-01-05 13:51 UTC (permalink / raw)
  Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 1915 bytes --]

This function has an interesting history.

Someone or something in #guile had clued me there was a magic
string-replace-substring somewhere in guile-user archives. With some luck I
found it, and had bookmarked it, and kept it at the back of my mind for a
good number of months.

Meanwhile I was steadily learning guile, and cleaning up GnuCash. Noting
GnuCash still supports use of guile-2.0, I was finally able to use Mark's
implementation to fix a bad one[1].   After discussing this change with
GnuCash's lead developer, we realised that upgrading the embedded guile in
GnuCash in Windows to 2.2 would solve a whole lot of i18n issues, which it
did.

Hence in a roundabout way, we are thankful for the original post and the
discussion[2] around it.

[1] https://github.com/Gnucash/gnucash/commit/7d15e6e4
[2] https://lists.gnu.org/archive/html/guile-devel/2014-03/msg00058.html

On Sun, 5 Jan 2020 at 12:12, Jan Nieuwenhuizen <janneke@gnu.org> wrote:

> Andy Wingo writes:
>
> > On Fri 20 Dec 2019 18:32, lloda <lloda@sarc.name> writes:
> >
> >> This patch adds string-replace-substring that wingo posted to the
> >> mailing list to (ice-9 strings). This is a commonly used function and a
> >> good implementation isn't trivial, so I think it deserves inclusion.
> >
> > I didn't know that this module existed :)  Sure, why not.  Please fix
> > the commit message when you push.
>
> That's great!  This is one of the things that has always amazed and
> annoyed me to be missing and I have written several half-baken
> implementations of it when I needed something like it.
>
> Would it be too much to ask for some documentation to go with it, so
> that we will be able to find it?
>
> Greetings,
> janneke
>
> --
> Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
> Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com
>
>

[-- Attachment #2: Type: text/html, Size: 2880 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add string-replace-substring to (ice-9 string-fun)
  2020-01-05 12:12   ` Jan Nieuwenhuizen
  2020-01-05 13:51     ` Christopher Lam
@ 2020-01-05 14:01     ` lloda
  2020-01-05 23:40       ` David Pirotte
  1 sibling, 1 reply; 8+ messages in thread
From: lloda @ 2020-01-05 14:01 UTC (permalink / raw)
  To: Jan Nieuwenhuizen; +Cc: Andy Wingo, guile-devel



> On 5 Jan 2020, at 13:12, Jan Nieuwenhuizen <janneke@gnu.org> wrote:
> 
> Andy Wingo writes:
> 
>> On Fri 20 Dec 2019 18:32, lloda <lloda@sarc.name> writes:
>> 
>>> This patch adds string-replace-substring that wingo posted to the
>>> mailing list to (ice-9 strings). This is a commonly used function and a
>>> good implementation isn't trivial, so I think it deserves inclusion.
>> 
>> I didn't know that this module existed :)  Sure, why not.  Please fix
>> the commit message when you push.
> 
> That's great!  This is one of the things that has always amazed and
> annoyed me to be missing and I have written several half-baken
> implementations of it when I needed something like it.
> 
> Would it be too much to ask for some documentation to go with it, so
> that we will be able to find it?

Sure, I'll include that in the fixed patch.

Regards

	Daniel




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add string-replace-substring to (ice-9 string-fun)
  2020-01-05 13:51     ` Christopher Lam
@ 2020-01-05 15:49       ` Arne Babenhauserheide
  0 siblings, 0 replies; 8+ messages in thread
From: Arne Babenhauserheide @ 2020-01-05 15:49 UTC (permalink / raw)
  To: guile-devel


Christopher Lam <christopher.lck@gmail.com> writes:

> This function has an interesting history.
>
> Someone or something in #guile had clued me there was a magic
> string-replace-substring somewhere in guile-user archives. With some luck I
> found it, and had bookmarked it, and kept it at the back of my mind for a
> good number of months.

To take the history further: I had initially started a very slow version
of string-replace-substring for wisp (because in the original wisp
parser which re-implemented something like read I needed
string-replacing all the time):
https://lists.gnu.org/archive/html/guile-devel/2013-09/msg00028.html

The original version looked like this (written on the Friday 13th in
September 2013):
define : string-replace-substring s substring replacement
       . "Replace every instance of substring in s by replacement."
       let : : sublen : string-length substring
           let replacer
               : newstring s
                 index : string-contains s substring
               if : not : equal? index #f
                  let : : replaced : string-replace s replacement index : + index sublen
                    replacer replaced : string-contains replaced substring
                  . newstring

;; ^
;; |
;; +---- horribly slow code! Do not copy!

It was afterwards improved with help by ijp to be 2x faster, and then by
Mark Weaver to be 80x faster (that’s not a typo: 80x faster!).

Half a year later Andy Wingo improved it further, replacing define* by
define, but I did not use it back then, because I needed full utf8 at
all times, regardless of the port encoding.

So the current version is at least 160x faster than my initial naive
implementation.

Thank you very much for picking it up and getting it into Guile!

Best wishes,
Arne
--
Unpolitisch sein
heißt politisch sein
ohne es zu merken



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add string-replace-substring to (ice-9 string-fun)
  2020-01-05 14:01     ` lloda
@ 2020-01-05 23:40       ` David Pirotte
  0 siblings, 0 replies; 8+ messages in thread
From: David Pirotte @ 2020-01-05 23:40 UTC (permalink / raw)
  To: lloda; +Cc: Andy Wingo, guile-devel

[-- Attachment #1: Type: text/plain, Size: 1142 bytes --]

Hi,

> > On 5 Jan 2020, at 13:12, Jan Nieuwenhuizen <janneke@gnu.org> wrote:
> > 
> > Andy Wingo writes:
> >   
> >> On Fri 20 Dec 2019 18:32, lloda <lloda@sarc.name> writes:
> >>   
> >>> This patch adds string-replace-substring that wingo posted to the
> >>> mailing list to (ice-9 strings). This is a commonly used function
> >>> and a good implementation isn't trivial, so I think it deserves
> >>> inclusion.  
> >> 
> >> I didn't know that this module existed :)  Sure, why not.  Please
> >> fix the commit message when you push.  
> > 
> > That's great!  This is one of the things that has always amazed and
> > annoyed me to be missing and I have written several half-baken
> > implementations of it when I needed something like it.
> > 
> > Would it be too much to ask for some documentation to go with it, so
> > that we will be able to find it?  
> 
> Sure, I'll include that in the fixed patch.

Fwiw, guix has a (different) implementation as well, I don't know which
is best, but I think it should be anmed string-replace-all

	http://git.savannah.gnu.org/cgit/guix.git/tree/guix/utils.scm

David.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add string-replace-substring to (ice-9 string-fun)
  2019-12-20 17:32 [PATCH] Add string-replace-substring to (ice-9 string-fun) lloda
  2020-01-05 11:34 ` Andy Wingo
@ 2020-01-06  9:08 ` Linus Björnstam
  1 sibling, 0 replies; 8+ messages in thread
From: Linus Björnstam @ 2020-01-06  9:08 UTC (permalink / raw)
  To: lloda, guile-devel

Did you try it with a "better" string-contains? IIRC the guile one is a naive string search (but in C) which could maybe be better if it was implemented as, say, a KMP search. The reference srfi-13 has one you could try. That could be faster for longer strings and longer patterns. 

If that is faster, I can port a constant-space one that I have in ocaml somewhere in the catacombs of my.computer.

-- 
  Linus Björnstam

On Fri, 20 Dec 2019, at 18:32, lloda wrote:
> 
> This patch adds string-replace-substring that wingo posted to the 
> mailing list to (ice-9 strings). This is a commonly used function and a 
> good implementation isn't trivial, so I think it deserves inclusion.
> 
> 
> 
> Attachments:
> * 0001-Add-string-replace-substring-to-ice-9-string-fun.patch



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-01-06  9:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-20 17:32 [PATCH] Add string-replace-substring to (ice-9 string-fun) lloda
2020-01-05 11:34 ` Andy Wingo
2020-01-05 12:12   ` Jan Nieuwenhuizen
2020-01-05 13:51     ` Christopher Lam
2020-01-05 15:49       ` Arne Babenhauserheide
2020-01-05 14:01     ` lloda
2020-01-05 23:40       ` David Pirotte
2020-01-06  9:08 ` Linus Björnstam

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