Euphoria
Ticket #371:
Duplicate method for finding and replacing?
-
Reported by
jeremy
Nov 12, 2010
These two methods are very similar:
What to do?
Details
1. Comment by SDPringle
Nov 12, 2010
One could document them better so we don't confuse them. There are times we want to precisely change equal members of a sequence from one needle to another but not subsequences that match a needle to another. You can put links between one and the other in the docs as one might decide one or the other is less than optimal and would like to use the other in place of it.
The find_replace documentation says it searches and replaces 'a needle.' It would be better to use the phrase 'all or up to limit instances of a needle.'
2. Comment by mattlewis
Nov 12, 2010
SDPringle said...
There are times we want to precisely change equal members of a sequence from one needle to another but not subsequences that match a needle to another.
What?
3. Comment by SDPringle
Nov 12, 2010
There are times we want to change members, s[i], of a sequence from one needle to another but not subsequences s[i..j] from that needle to another.
4. Comment by mattlewis
Nov 12, 2010
Ok, but I don't think that's the issue here.
5. Comment by jimcbrown
Nov 13, 2010
The two methods look similar but they are different. My conclusion - they provide different but similar and overlapping functionality, and we need both, even though they are easily confusable.
Compare and contrast:
include std/pretty.e
include std/search.e
include std/sequence.e
sequence s
s = find_replace('/', "/euphoria/demo/unix", "\\", 2)
pretty_print(1, s)
s = replace_all("/euphoria/demo/unix", '/', "\\")
pretty_print(1, s)
Perhaps this should become a "better docs" ticket?
6. Comment by jeremy
Nov 13, 2010
Add in to Jim's example, match_replace()
s = match_replace('/', "/euphoria/demo/unix", "\\", 2)
pretty_print(1, s)
I personally think find_replace and match_replace should remain and replace_all be removed. find/match fits out existing use not only in naming but in parameter order, i.e. needle, haystack. replace_all doesn't fit naming or our parameter order, i.e. it is haystack, needle.
7. Comment by jeremy
Nov 13, 2010
creole.e seems to be the only program in my source tree using it, including euweb, webclay and edbi. Others could be using it of course but I do think it's presence is a bug.
8. Comment by jimcbrown
Nov 13, 2010
I've discovered that by replacing replace_all() with match_replace(), every test but one in t_sequence.e passes
The one test that fails, on line 575, is replace_all("abracadabra", "", "X")
However, that case is a trivial one, and can be replaced by either using find_replace() or just simply returning the haystack when the needle is "".
9. Comment by jimcbrown
Nov 13, 2010
We're stuck with replace_all() (for the same reason we're stuck with match_from()) but I've at least reimplemented it so that it's obvious that match_replace() and replace_all() do the same thing.
10. Comment by jeremy
Nov 13, 2010
Why are we stuck with it? I know we've released it under RC1, but I think it's a pretty serious bug. Think of 4.0, 4.1, ... all dealing with "What about this func vs. this func?"
It's an easy change for people to just use find_replace/match_replace.
11. Comment by jeremy
Nov 13, 2010
See: hg:euweb/rev/93ed8d027fbe
Converted from replace_all -> match_replace, ticket:371
12. Comment by jeremy
Nov 14, 2010
replace_all() has been removed from Euphoria.
13. Comment by ne1uno
Dec 03, 2010
reopen, release notes missing? sorry if I missed it.
replace_all -> match_replace
14. Comment by jimcbrown
Dec 03, 2010
Reopened as a documentation ticket, to add to the release notes.
15. Comment by jeremy
Dec 03, 2010
It's was there, under Enhancements/Changes. I think later (possibly) we added the "Removed" category. I moved it there. Good catch.
16. Comment by ne1uno
Dec 03, 2010
not to quibble, remove_all is still valid, and appears in the removed section.
replace_all is the function that had been removed.
17. Comment by jeremy
Dec 03, 2010
Oh my! That's not quibbling, its clearly wrong. Thanks for catching that.
18. Comment by jeremy
Dec 03, 2010
New docs uploaded.
GitHub