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.

Sign up for GitHub

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

Jump to bottom

Fix issue parsing block comments, and always retain prefix comments for cgo imports. #49

Merged
merged 3 commits into from
Mar 3, 2022
Merged

Fix issue parsing block comments, and always retain prefix comments for cgo imports. #49

merged 3 commits into from
Mar 3, 2022

Conversation

charleskorn
Copy link
Contributor

Resolves #48.

Comments before import "C" should always be retained, as these are used by cgo.

@charleskorn
Fix "Error: an error occured while trying to parse imports: path is m…
66fd3f5 
…issing starting quotes" when parsing a file with block comments.

Fixes daixiang0#48.

Note that this only handles simple cases for block comments, and does
not cover more complex cases (see FIXME in parse.go).

Signed-off-by: Charles Korn <me@charleskorn.com>
@charleskorn
Make block comment parsing more robust to different formatting.
de8110b 
Signed-off-by: Charles Korn <me@charleskorn.com>
@charleskorn
Copy link
Contributor Author

parseToImportDefinitions is now quite large, if you have any suggestions on how to break it down, I'd be keen to hear them.

@daixiang0
Copy link
Owner

daixiang0 commented Mar 2, 2022 •
edited
Loading

Thanks for your contribution!

Could we use "C" to detect CGO and make it into a single func?
I prefer divide parseToImportDefinitions into some small parts to match different cases.

@charleskorn
Copy link
Contributor Author

Could we use "C" to detect CGO and make it into a single func?
I prefer divide parseToImportDefinitions into some small parts to match different cases.

Block comments (/* ... */) aren't limited to cgo imports - they're equally valid elsewhere.

@charleskorn
Add test case for multiple comments before a "C" import.
d5b59c3 
Signed-off-by: Charles Korn <me@charleskorn.com>
@charleskorn
Copy link
Contributor Author

It might be more robust to use the ast package to parse files than doing what I've done here, but that would likely be a far bigger change.

@daixiang0
Copy link
Owner

It might be more robust to use the ast package to parse files than doing what I've done here, but that would likely be a far bigger change.

Track as #50

@daixiang0 daixiang0 merged commit ec40303 into daixiang0:master Mar 3, 2022
@charleskorn
Copy link
Contributor Author

@daixiang0 thanks for merging this. Any chance you could tag a new release with this fix included?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daixiang0 daixiang0 daixiang0 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Error: an error occured while trying to parse imports: path is missing starting quotes" on valid Go file
2 participants
@charleskorn @daixiang0