Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

re.replace("foo", re"", "-") returns -foo, inconsistent with nre.replace #9437

Closed
kaushalmodi opened this issue Oct 18, 2018 · 3 comments · Fixed by #17546
Closed

re.replace("foo", re"", "-") returns -foo, inconsistent with nre.replace #9437

kaushalmodi opened this issue Oct 18, 2018 · 3 comments · Fixed by #17546

Comments

@kaushalmodi
Copy link
Contributor

kaushalmodi commented Oct 18, 2018

Detailed comment: #9315 (comment)

/cc @skilchen

EDIT by @timotheecour

import re
echo replace("foo", re"", "-")
# -foo

copying over the comment from @skilchen :

import re
echo replace("foo", re"", "-")

produces:
-foo
While in the common understanding of regular expressions there is an empty string in front of any string, one between every substring of length 1 and one at the end of the string.
nre.nim produces the usually expected -f-o-o-
With nre as well as in the other languages I know the following is also possible:

import nre
echo replace("foo", re"^|$", "-")

which produces
-foo-
as expected, but using re you get: -foo which doesn't seem correct.

import re
echo "".replacef(re"([^\d]*)(\d*)([^\w]*)", "$1 - $2 - $3")

still produces an empty line, which doesn't seem correct, I would expect - -. However on a string that usually should be split using this pattern, it works:

import re
echo "abc123###".replacef(re"([^\d]*)(\d*)([^\w]*)", "$1 - $2 - $3")

correctly produces:
abc - 123 - ###


further discussion

There is no wrong logic here, we don't claim to be bug-compatible with other regex engines.

if we're departing from other regex engines, an option could be to throw ; but at least it'd be nice to have consistent behavior across re,nre,nim-regex

@krux02
Copy link
Contributor

krux02 commented Oct 20, 2018

Can you please copy the detailed comment here into the issue where it is visible.

And please don't use imperative in the issue title, call the problem it present tense. This also applies to your other issues.

@timotheecour timotheecour changed the title Fix the replace("foo", re"", "-") case in re module replace("foo", re"", "-") returns -foo, inconsistent with nre Oct 29, 2018
@timotheecour timotheecour changed the title replace("foo", re"", "-") returns -foo, inconsistent with nre re.replace("foo", re"", "-") returns -foo, inconsistent with nre.replace Oct 29, 2018
@timotheecour
Copy link
Member

timotheecour commented Oct 29, 2018

@krux02 done (@kaushalmodi I took the liberty to edit your post)

@timotheecour
Copy link
Member

timotheecour commented Mar 27, 2021

@xflywind thanks for self-assigning this; maybe also define -d:nimLegacyReReplace for backward compatibility in your PR?

ringabout added a commit to ringabout/Nim that referenced this issue Mar 28, 2021
Araq pushed a commit that referenced this issue Jun 10, 2021
* fix nim js cmp fails at CT

* fix
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants