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

Single import becomes a list of imports #144

Closed
Ericwww opened this issue Dec 11, 2023 · 5 comments · Fixed by #145
Closed

Single import becomes a list of imports #144

Ericwww opened this issue Dec 11, 2023 · 5 comments · Fixed by #145

Comments

@Ericwww
Copy link
Contributor

Ericwww commented Dec 11, 2023

Hi @incu6us

Sorry to bother you again.

I would like to use goimports-reviser as a replacement for goimports.

But at the single import situation, their behavior is inconsistent.

For example,

package testdata

import "log"

func AAA() {
	log.Print("aaa")
}

When I run goimports, the result is the same as above.

But when I run goimports-reviser, the result will be

package testdata

import (
	"log"
)

func AAA() {
	log.Print("aaa")
}

A single import becomes a list of imports, but there is only one import in the list.

I think the problem is at reviser/file.go:378, function: #rebuildImports

@Ericwww
Copy link
Contributor Author

Ericwww commented Dec 11, 2023

I'm trying to fix it. All unit tests (including the ones I added) passed.
But I'm not sure if this modification is correct.

@incu6us
Copy link
Owner

incu6us commented Dec 13, 2023

thanks @Ericwww
I'll check

Ericwww added a commit to Ericwww/goimports-reviser that referenced this issue Jan 6, 2024
@l2cup
Copy link

l2cup commented Jan 23, 2024

Can i bump this? We're blocked on upgrading our version until this this is fixed.

Thanks 🙏

Ericwww added a commit to Ericwww/goimports-reviser that referenced this issue Jan 23, 2024
Ericwww added a commit to Ericwww/goimports-reviser that referenced this issue Jan 23, 2024
Ericwww added a commit to Ericwww/goimports-reviser that referenced this issue Jan 23, 2024
@Ericwww
Copy link
Contributor Author

Ericwww commented Jan 23, 2024

Hi @l2cup, I'm not the owner of this repository.

I just submitted my new code. PR: #145
I think such logic should be more elegant than before and can cover more scenes.
I have passed all the checks except Build.

If @incu6us thinks there is no problem, I think this issue can be closed. 😃 😃

@incu6us
Copy link
Owner

incu6us commented Jan 24, 2024

thanks for your efforts @Ericwww
it is merged and available in v3.6.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants