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

Multiple prefix() sections should not be automatically sorted #205

Closed
SOF3 opened this issue Apr 8, 2024 · 5 comments · Fixed by #210
Closed

Multiple prefix() sections should not be automatically sorted #205

SOF3 opened this issue Apr 8, 2024 · 5 comments · Fixed by #210
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@SOF3
Copy link

SOF3 commented Apr 8, 2024

What version of GCI are you using?

0.13.4

Reproduce Steps

$ example.go
package example

import (
        _ "example.com/a/b"
        _ "example.com/i/j"
        _ "example.com/m/n"
        _ "example.com/p/q"
)

$ go run github.com/daixiang0/[email protected] print example.go \
>       -s standard
>       -s default
>       -s 'prefix(example.com/i/j,example.com/m/n)'
>       -s 'prefix(example.com/a/b,example.com/p/q)'
package example

import (
        _ "example.com/a/b"
        _ "example.com/p/q"

        _ "example.com/i/j"
        _ "example.com/m/n"
)

What did you expect to see?

The sections should be consistent with the order of -ss passed.

What did you see instead?

The sections follow lexicographical order of the prefix() string. If I change the fourth section to prefix(example.com/p/q,example.com/a/b), the two sections in the output will be swapped.

@SOF3
Copy link
Author

SOF3 commented Apr 8, 2024

Possible workaround for now: add nosuchmodule/1, and nosuchmodule/2, after prefix( to force the order of prefixes.

@daixiang0
Copy link
Owner

The two prefix section does not follow lexicographical order which is written in the README, the strings in the prefix follow, so you can put then into one prefix section.

I think it is good to improve, you can take it if interested.

Label it as help wanted if someone else interested.

@daixiang0 daixiang0 added help wanted Extra attention is needed good first issue Good for newcomers labels Apr 8, 2024
@SOF3
Copy link
Author

SOF3 commented Apr 8, 2024

I don't understand. What is written in the readme? And what does "you can put them into one prefix section" mean? I am referring to multiple prefix sections, not multiple prefixes within the same section; the , part is just for illustration.

@DanWlker
Copy link
Contributor

DanWlker commented Jul 7, 2024

Hi, can i have a go on this issue?

@asfaltboy
Copy link

asfaltboy commented Jul 8, 2024

I came across this issue, and I was missing the "--custom-order" flag, @SOF3 are you providing it?

It seems to be required when sections are given, otherwise sections are sorted alphabetically.

I found it a tad confusing, and it took me a while to figure out that without it, just adding sections in a given order has no impact on order of sections.

My suggestion? Make the behaviour of the flag true by default, as it's implicitly expected. Then, for cases where people would prefer to add custom sections, but rely on GCI to sort them, add a "--sort-sections" flag and explain it's behaviour clearly in the docs (which sections exactly will appear in what order, and which imports will be lexicographically sorted)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants