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

automatically detect Go module path #31

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dmke
Copy link

@dmke dmke commented Jan 26, 2021

This is just a proof of concept for #30.

pkg/gci/mod.go Outdated Show resolved Hide resolved
pkg/gci/mod.go Outdated Show resolved Hide resolved
@victoraugustolls
Copy link

Hey guys! @dmke would you be interested into updating this PR?

Also @daixiang0 what is your take here? If you don't mind me asking! I'm working with a monorepo with multiple modules, and this feature would make my life simpler!

@dmke
Copy link
Author

dmke commented Feb 10, 2022

@victoraugustolls Interested sure, but I'm currently in short supply of free time :)

It might take me until the weekend before I get to update the PR. Looking at #43, this might not be a trivial task...

@daixiang0
Copy link
Owner

This is an useful feature I think :)

@daixiang0
Copy link
Owner

Thanks for your contribution, now it has changed a lot.

I will review if you update it.

This started as a proof-of-concept for daixiang0#30.

Signed-off-by: Dominik Menke <dom@digineo.de>
@dmke
Copy link
Author

dmke commented May 30, 2022

I've updated the code, however I can't get it to work:

At some point, (pkg/gci/sections).Module needs to know which module paths are in scope. These are currently is seeded in (*pkg/gci/GciConfiguration).InitializeModules, however, because (*pkg/gci/GciConfiguration).Sections is a SectionList, it is difficult to inject the module paths (for _, section := range g.Sections creates a copy). The added test case fails for this reason.

I think a better way to pass the module path is to add an appropriate field to (pkg/gci/imports).ImportDef, although I couldn't figure out how/when those objects are created.

Looking at (pkg/gci).processStdInAndGoFilesInPaths(), there's another lingering problem: gci can read file contents from stdin, but the module path resolver needs a file name in order to traverse the file system and find the associated go.mod. This might produce different results:

$ cat ../whatever.go | gci diff -s Standard,Module,Default
$ gci diff -s Standard,Module,Default ../whatever.go

for path := range knownModulePaths {
modulePaths = append(modulePaths, path)
}
moduleSection.SetModulePaths(modulePaths)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work, moduleSection is a pointer to a copy of g.Section[i].

@daixiang0
Copy link
Owner

I think this feature is helpful for a project, we can detect from go.mod. But for a file, it is hard to detect.

Maybe we can skip single file support for now, until we get a better solution.

@dmke
Copy link
Author

dmke commented Jul 6, 2022

Feel free to take my code and make it work. I currently am quite busy with other things, and don't know when I gain the mental capacity needed to complete this...

@F0rzend
Copy link

F0rzend commented Oct 12, 2023

I usually call gci using file watcher from the root of the project, indicating the full path to the file. Therefore, I think that you can try to look for the go.mod file in the current folder (where the utility is called) and if found, use it to determine the name of the module

@matthewhughes934
Copy link
Contributor

Feel free to take my code and make it work. I currently am quite busy with other things, and don't know when I gain the mental capacity needed to complete this...

Thanks for your work! I've created #179 that is a slimmed-down version of this work: it just handles looking at go.mod in the current directory (the goal is to try and implement just the bare-minimum for this feature, and add extra functionality when/if we want it)

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 this pull request may close these issues.

None yet

6 participants