Skip to content
This repository was archived by the owner on Jul 15, 2023. It is now read-only.

goImports: avoid collapsing new imports into pseudo import line#3045

Merged
ramya-rao-a merged 2 commits intomicrosoft:masterfrom
hyangah:import
Apr 27, 2020
Merged

goImports: avoid collapsing new imports into pseudo import line#3045
ramya-rao-a merged 2 commits intomicrosoft:masterfrom
hyangah:import

Conversation

@hyangah
Copy link
Copy Markdown
Contributor

@hyangah hyangah commented Feb 13, 2020

getTextEditForAddImport tries to insert a newly added package
into an existing import group. If no import group exists, it
tries to merge single line import statements and create a group.

In this process, import statements like

import "C"

are pseudo imports and shouldn't be grouped with other imports.
This CL changes parseFilePrelude to detect such pseudo imports
and excludes such imports from the grouping logic.

Fixes #1701

@hyangah
Copy link
Copy Markdown
Contributor Author

hyangah commented Feb 13, 2020

the test failures on platforms other than mac are real - I messed up with file name (case sensitivity). Will fix it when I can access my linux tomorrow.

hyangah added 2 commits April 20, 2020 17:53
getTextEditForAddImport tries to insert a newly added package
into an existing import group. If no import group exists, it
tries to merge single line import statements and create a group.

In this process, import statements like

  import "C"

are pseudo imports and shouldn't be grouped with other imports.
This CL changes parseFilePrelude to detect such pseudo imports
and excludes such imports from the grouping logic.

Fixes microsoft#1701
mac and vsccode ignored casing, but linux and git were unhappy.
Comment thread src/util.ts
ret.imports.push({ kind: 'multi', start: i, end: -1, pkgs: [] });
}
if (line.match(/^(\s)*import(\s)+[^\(]/)) {
} else if (line.match(/^\s*import\s+"C"/)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is C the only import that falls under this pseudo category?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. "C" is the only one in this category (psuedo import whose placement in the import affects the correctness/functionality)

@ramya-rao-a ramya-rao-a merged commit c565b21 into microsoft:master Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

automatic import adder should not separate first import from its comment

2 participants