Support JSON Marshal/Unmarshal #77
Conversation
regexp.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| re.muRun.Lock() |
There was a problem hiding this comment.
Why lock and unlock the runner pool?
There was a problem hiding this comment.
If the variable value of the struct is changed while the runner is running, it can cause some unexpected results
There was a problem hiding this comment.
Ok, I don't think the framework regexp version of this function is safe to do while a regexp may be running. Is there a use case where you'd need to Unmarshal into a running regex?
regexp.go
Outdated
| } | ||
| re.muRun.Lock() | ||
| defer re.muRun.Unlock() | ||
| re.pattern = newRE.pattern |
There was a problem hiding this comment.
I'd prefer the more elegant *re = *newRE style copy, so lets change Regexp.muRun to a pointer (and update Compile) so we can copy the struct with go vet warnings.
regexp.go
Outdated
| // UnmarshalText implements [encoding.TextUnmarshaler] by calling | ||
| // [Compile] on the encoded value. | ||
| func (re *Regexp) UnmarshalText(text []byte) error { | ||
| newRE, err := Compile(string(text), re.options) |
There was a problem hiding this comment.
I think the options should be 0 here since there's no real valid way to set re.options anyway.
There was a problem hiding this comment.
I'm also thinking about this. Is it possible to provide a global flag, such as DefaultUnmarshalOptions?
regexp.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| re.muRun.Lock() |
There was a problem hiding this comment.
Ok, I don't think the framework regexp version of this function is safe to do while a regexp may be running. Is there a use case where you'd need to Unmarshal into a running regex?
regexp.go
Outdated
| // UnmarshalText implements [encoding.TextUnmarshaler] by calling | ||
| // [Compile] on the encoded value. | ||
| func (re *Regexp) UnmarshalText(text []byte) error { | ||
| newRE, err := Compile(string(text), re.options) |
|
ready to go |
resolve #76