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

Jump to bottom

x/tools/gopls: ignore line directives #55043

Closed
findleyr opened this issue Sep 13, 2022 · 5 comments
Closed

x/tools/gopls: ignore line directives #55043

findleyr opened this issue Sep 13, 2022 · 5 comments
Assignees
@pjweinb @adonovan
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/backlog

Comments

@findleyr
Copy link
Contributor

findleyr commented Sep 13, 2022 •
edited

From a discussion with @pjweinb and @adonovan:

The existence of line directives causes gopls all sorts of problems, because it means that there are two potentially valid source positions for a token.Pos, and it's not always clear which is correct in context. It also means that we need to access multiple files from operations that would otherwise be local to a single file. This has been a source of historical (and in all likelihood ongoing) bugs.

This issue records an outcome from that discussion: we should ignore line directives internally to gopls (in particular, being careful to call token.File.PositionFor instead of token.File.Position). If this causes usability issues in e.g. CGo programs, we should address those issues individually. For example, we could perform a post-processing pass on diagnostics that duplicates them to their mapped positions.

CC @golang/tools-team
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Sep 13, 2022
@gopherbot gopherbot added this to the Unreleased milestone Sep 13, 2022
@findleyr findleyr modified the milestones: Unreleased, gopls/later Sep 13, 2022
@adonovan
Copy link
Member

We could even offer an operation that jumps the cursor to the image of the current position through any applicable //line mapping.

@findleyr findleyr mentioned this issue Sep 16, 2022
Closed
@pjweinb pjweinb self-assigned this Oct 17, 2022
@gopherbot
Copy link

Change https://go.dev/cl/457656 mentions this issue: gopls/internal/lsp: drop support for line directives

@adonovan adonovan self-assigned this Dec 14, 2022
@kortschak
Copy link
Contributor

https://go.dev/cl/439115 is marked as having addressed this (with a digit transpose). Should this be closed?

@pjweinb
Copy link

pjweinb commented Dec 18, 2022 via email

@adonovan
Copy link
Member

It's fine to close this issue since @pjweinb your CL addressed it. My CL 457656 is now just the some follow-up simplifications to location processing (with more to follow).

@golang golang locked and limited conversation to collaborators Dec 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@pjweinb pjweinb

@adonovan adonovan

Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/backlog
Development

No branches or pull requests
5 participants
@kortschak @pjweinb @adonovan @gopherbot @findleyr