Skip to content

Conversation

@emcfarlane
Copy link
Contributor

@emcfarlane emcfarlane commented Oct 30, 2025

This PR adds support for goto definition based on the cursor position in the fields FullName. For example: google.protobuf.Timestamp can go to both the type Timestamp or the package google.protobuf within the file google/protobuf/timestamp.proto. We may wish to extend this behaviour to also cover going to every file within the package google.protobuf.

@github-actions
Copy link
Contributor

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedOct 30, 2025, 2:35 PM

Copy link
Member

@stefanvanburen stefanvanburen left a comment

Choose a reason for hiding this comment

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

two things, and one weird thing I noticed:

  • I think hover should have the same logic as "go-to definition"; that is, if I'm hovering on a symbol and get docs, I would expect go-to definition to bring me to the location where that doc comment comes from. (I think we should add this logic to hover.)
  • The UX feels a little weird on the parent types, given the option (buf.validate.field).string.max_len, using go-to-definition:
    • max_len does sorta what I expect (it brings me to BytesRules.max_len; it should bring me to StringRules.max_len 😅)
    • string does sorta what I expect (it brings me to BytesRules, which also defines a max_len, but should bring me to StringRules)
    • field takes me to the package buf.validate line in validate.proto;
  • (likewise with google.protobuf.Timestamp, I'm not sure if it's better UX that google doesn't do anything and protobuf just brings me to a file, rather than have the whole thing go to Timestamp?)

@emcfarlane
Copy link
Contributor Author

Closing in favour of breaking the symbols up, cc @doriable

@emcfarlane emcfarlane closed this Nov 12, 2025
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.

3 participants