overhaul regexp package #130
Conversation
regexp/regexp.go
Outdated
|
|
||
| // Converts the replacement pattern to make it compatible with what the regexp package expects. | ||
| func convertReplacementPattern(repl string) string { | ||
| // Escapes the specifal dollar characters if any |
| // Converts the replacement pattern to make it compatible with what the regexp package expects. | ||
| func convertReplacementPattern(repl string) string { | ||
| // Escapes the specifal dollar characters if any | ||
| repl = strings.ReplaceAll(repl, "$", "$$") |
There was a problem hiding this comment.
Does this always work? I think it's over-zealous, and would break this example:
import re
s = 'big$money'
re.match(r'[\w]+\$[\w]+', s)
Rewriting a regex to work with a different syntax probably requires fully parsing it, not just single character replacement. Of course, it's a bit of an edge case, and if we're ok with losing correctness until the upstream version is done, it's probably not too big of a problem.
| func (r *Regexp) AttrNames() []string { return builtinAttrNames(regexMethods) } | ||
|
|
||
| var regexMethods = map[string]*starlark.Builtin{ | ||
| "find": starlark.NewBuiltin("find", find), |
There was a problem hiding this comment.
A big question we keep coming back to: how much do we want to be just like python? The more our library can be a drop-in replacement for python, the easier it will be for users to get comfortable with starlark. In particular, I always use the match method in python, as most docs and tutorials reference it early on. Should we be implementing that as well?
There was a problem hiding this comment.
Yeah, we should have this chat before merging a breaking change like this. Let's leave this PR in limbo for the moment
this code is a shim while we work to merge the old re library (no heavily modified) upstream into starlark/lib BREAKING CHANGE: re is deprecated in favour of new regexp package
I'd like to harmonize our API around what's coming down the pipe for starlark/lib, but google/starlark-go#369 has been open since April, so let's shim this in until it's merged
closes #91