-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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
x/tools/imports: consider exporting fixImports #23782
Comments
FWIW, the cost of this change isn't measured in characters or lines modified. It's a question of how much ongoing maintenance cost there is keeping this a stable interface for people to use long-term and whether that's even the API we want people using. |
I understand the trade-offs and for my case think it is worth it (obviously). It seems like it follows in the spirit of the module, just at a lower level and the API for that function is very direct to work with when dealing with ASTs. Thank you for considering it. |
If deemed a good change I'll be happy to submit a pull request with the change to help things along. I plan on maintaining this as a fork in the meantime, so I'll have it ready. Thanks again. |
Went ahead and created the Pull Request to facilitate the decision. |
This issue seems related to #12696. Although I don't know how exporting only The PR doesn't seem to help facilitate the decision, because it doesn't communicate why It would help if you provided more information about how it'd be used, etc. |
The use case is when you want to update the imports in the AST without the text. For example I'm writing a custom formatter, to format code in a presentation like style, and wanted the import fix. I considered forking or just copying out the needed code, but thought it might be generally useful and it'd be better to not duplicate the functionality if unnecessary. |
We have exposed an internal interface to the imports package that might suit your needs, but I don't think we're going to make it public. Feel free to fork |
This is a 3 character change that would make this module much more useful for people working on custom formatting solutions. Without this you need to format the code and then re-parse it to get the ast with the fixed imports which is very inefficient.
What version of Go are you using (
go version
)?1.9/tip
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?N/A
What did you do?
Wanted to fix the imports at the ast.File level using the x/tools/imports lib.
What did you expect to see?
fixImports should be FixImports so it is public and can be used when working with ast's vs. files.
What did you see instead?
It is not public and I am forced to fork the project to use it.
The text was updated successfully, but these errors were encountered: