Allow non capturing groups #9
Conversation
flags.go
Outdated
| return errors.New("import flag must be of form path:alias") | ||
| } | ||
|
|
||
| v[spl[0]] = spl[1] | ||
| v[strings.Join(spl[:len(spl)-1], ":")] = spl[len(spl)-1] |
There was a problem hiding this comment.
would it work to use strings.LastIndex here to avoid the join? something like:
i := strings.LastIndex(val, ":")
if i < 1 {
return errors.New("import flag must be of form path:alias")
}
v[val[:i]] = val[i+1:]
tomfeigin
force-pushed
the
allow-non-capturing-groups
branch
from
38c63a0
to
81f7cac
Compare
tomfeigin
force-pushed
the
allow-non-capturing-groups
branch
from
81f7cac
to
643145a
Compare
|
@julz anything I can do for this PR to be merged? |
There was a problem hiding this comment.
oops! huge apologies @tomfeigin - this got completely lost in my GitHub notifications 😬
analyzer_test.go
Outdated
|
|
||
| a := makeAnalyzer() | ||
| flg := a.Flags.Lookup("alias") | ||
| assert.EqualError(t, flg.Value.Set(""), errWrongAlias.Error()) |
There was a problem hiding this comment.
could we avoid adding a dependency on testify here? I think just asserting that we get an error might be fine for this test
analyzer_test.go
Outdated
| } | ||
|
|
||
| t.Run("wrong usage", func(t *testing.T) { |
There was a problem hiding this comment.
I'd either move this test to its own top-level function (e.g. TestIncorrectFlags), or add it to the existing table in this test (e.g. by adding an expectAliasFlagErr field to the rows). I'd also personally be fine if we exported the stringMap type and relied on a unit test (without constructing an analyser) for that to test this case.
tomfeigin
force-pushed
the
allow-non-capturing-groups
branch
from
643145a
to
ad1d901
Compare
analyzer_test.go
Outdated
| } | ||
|
|
||
| func TestIncorrectFlags(t *testing.T) { | ||
| t.Run("wrong usage", func(t *testing.T) { |
There was a problem hiding this comment.
don't need the t.Run since there's only one test here :)
analyzer_test.go
Outdated
| t.Run("wrong usage", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assertWrongAliseErr := func(msg string, err error) { |
There was a problem hiding this comment.
| assertWrongAliseErr := func(msg string, err error) { | |
| assertWrongAliasErr := func(msg string, err error) { |
Fix the flags parsing to accept regexes with non capturing groups Signed-off-by: Tom <[email protected]>
tomfeigin
force-pushed
the
allow-non-capturing-groups
branch
from
ad1d901
to
c3e0594
Compare
|
Thanks @tomfeigin! |
|
hi, would you please be so kind as to make a new release including this. thx |
Fix the flags parsing to accept regexes with non capturing groups
Changes
/kind bug
Fixes #8
Regexs with non-capturing groups didn't work
Release Note
Docs