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.

Sign up for GitHub

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

Jump to bottom

Support multiple prefixes to group them together and/or a regex #107

Closed
polds opened this issue Aug 30, 2022 · 1 comment · Fixed by #142
Closed

Support multiple prefixes to group them together and/or a regex #107

polds opened this issue Aug 30, 2022 · 1 comment · Fixed by #142
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@polds
Copy link

polds commented Aug 30, 2022 •
edited
Loading

We have code hosted at alt.code.company.com and code.company.com It'd be nice to be able to provide prefix(alt.code.company.com,code.company.com) or a regular expression to match it.

So given:

import (
   "fmt"
   "github.com/daixiang0/gci"
   "code.company.com/foo/bar/baz"
   "alt.code.company.com/foo/boo"
)

The output of

gci write -s standard -s default -s 'Prefix(alt.code.company.com,code.company.com)' --custom-order --skip-generated .

Should yield:

import (
  "fmt"

  "github.com/daixiang0/gci"

  "alt.code.company.com/foo/boo"
  "code.company.com/foo/bar/baz"
)
@daixiang0
Copy link
Owner

Each custom group will be split, which is designed.

@yujiachen-y yujiachen-y mentioned this issue Dec 14, 2022
Open
luw2007 added a commit to luw2007/gci that referenced this issue Jan 16, 2023
@luw2007
Support multiple prefixes to group them together and/or a regex daixi…
e1d4e30 
…ang0#107
luw2007 added a commit to luw2007/gci that referenced this issue Jan 16, 2023
@luw2007
Support multiple prefixes to group them together and/or a regex daixi…
8db20a1 
…ang0#107

Signed-off-by: Author Name <luw2007@gmail.com>
luw2007 added a commit to luw2007/gci that referenced this issue Jan 16, 2023
@luw2007
Support multiple prefixes to group them together and/or a regex daixi…
d4ea0e0 
…ang0#107

Signed-off-by: luwei.will <luw2007@gmail.com>
@daixiang0 daixiang0 mentioned this issue Jan 17, 2023
Closed
@daixiang0 daixiang0 added enhancement New feature or request help wanted Extra attention is needed labels Feb 21, 2023
ccoVeille pushed a commit to ccoveille-forks/daixiang0-gci that referenced this issue Feb 25, 2023
@luw2007 @ccoVeille
Support multiple prefixes to group them together and/or a regex daixi…
d22c529 
…ang0#107

Signed-off-by: luwei.will <luw2007@gmail.com>
@ccoVeille ccoVeille mentioned this issue Feb 25, 2023
Merged
ccoVeille added a commit to ccoveille-forks/daixiang0-gci that referenced this issue Feb 25, 2023
@ccoVeille
Support multiple prefixes to group them together daixiang0#107
ec8136d
ccoVeille added a commit to ccoveille-forks/daixiang0-gci that referenced this issue Feb 25, 2023
@ccoVeille
Support multiple prefixes to group them together daixiang0#107
43e6c14 
Signed-off-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
ccoVeille added a commit to ccoveille-forks/daixiang0-gci that referenced this issue Feb 27, 2023
@ccoVeille
Support multiple prefixes to group them together daixiang0#107
223cca5 
Signed-off-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
ccoVeille added a commit to ccoveille-forks/daixiang0-gci that referenced this issue Feb 27, 2023
@ccoVeille
Support multiple prefixes to group them together daixiang0#107
790e621 
Signed-off-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
ccoVeille added a commit to ccoveille-forks/daixiang0-gci that referenced this issue Feb 28, 2023
@ccoVeille
Support multiple prefixes to group them together daixiang0#107
b0fe436 
This changes allows to use this to group multiple prefixes

gci write -s standard -s default -s 'Prefix(alt.code.company.com,code.company.com)' --custom-order --skip-generated

In order to provide this, a change was needed in gci --section parameter parsing
The code was using pflag.StringSliceP for parsing sections.

Please consider the pflag documentation:

> Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly.
> For example: --ss="v1,v2" --ss="v3"

While the usage of StringSliceP was legitimate with -local legacy parameter, if we consider gci documentation:

> -l, --local strings   put imports beginning with this string after 3rd-party packages, separate imports by comma

We can assume that when gci was rewritten in its "new style", it appears it was unintended to keep using pflag.StringSliceP.

Switching back to StringArrayP allows us to use comma in CLI arguments,
without this change the following command would have failed:

gci write -s standard -s default -s 'Prefix(alt.code.company.com,code.company.com)' --custom-order --skip-generated

because pflag.StringSliceP would have considered it as:

gci write -s standard -s default -s 'Prefix(alt.code.company.com' -s 'code.company.com)' --custom-order --skip-generated

We can assume this change will be OK. But it will break the following
command:

gci write -s standard,default

It was working because of StringSliceP

The syntax is now gci write -s standard -s default as documented

Signed-off-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
ccoVeille added a commit to ccoveille-forks/daixiang0-gci that referenced this issue Mar 1, 2023
@ccoVeille
Support multiple prefixes to group them together daixiang0#107
72599ab 
This changes allows to use this to group multiple prefixes

gci write -s standard -s default -s 'Prefix(alt.code.company.com,code.company.com)' --custom-order --skip-generated

In order to provide this, a change was needed in gci --section parameter parsing
The code was using pflag.StringSliceP for parsing sections.

Please consider the pflag documentation:

> Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly.
> For example: --ss="v1,v2" --ss="v3"

While the usage of StringSliceP was legitimate with -local legacy parameter, if we consider gci documentation:

> -l, --local strings   put imports beginning with this string after 3rd-party packages, separate imports by comma

We can assume that when gci was rewritten in its "new style", it appears it was unintended to keep using pflag.StringSliceP.

Switching back to StringArrayP allows us to use comma in CLI arguments,
without this change the following command would have failed:

gci write -s standard -s default -s 'Prefix(alt.code.company.com,code.company.com)' --custom-order --skip-generated

because pflag.StringSliceP would have considered it as:

gci write -s standard -s default -s 'Prefix(alt.code.company.com' -s 'code.company.com)' --custom-order --skip-generated

We can assume this change will be OK. But it will break the following
command:

gci write -s standard,default

It was working because of StringSliceP

The syntax is now gci write -s standard -s default as documented

Signed-off-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
daixiang0 pushed a commit that referenced this issue Mar 2, 2023
@ccoVeille
Support multiple prefixes to group them together #107 (#142)
65b147e 
This changes allows to use this to group multiple prefixes


Signed-off-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feature: support multiple prefixes ccoveille-forks/daixiang0-gci
2 participants
@polds @daixiang0