Skip to content

Do not collect trailing comments of previous lines #3563

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

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

jesse-shopify
Copy link
Contributor

@jesse-shopify jesse-shopify commented May 30, 2025

A discussion revealed that a better path forward is to associate comments to nodes in Prism and use that information in the LSP. That could replace this PR or be a follow-up.

Motivation

  • Comments for ivars were including trailing comments from previous lines. With RBS, these trailing comments are often RBS typing information.
  • In general, a trailing comment is assumed to be for the line it is on, not subsequent lines

Implementation

  • When collecting comments, encountering a trailing comment can stop the collection because no other comments will be for the current node

Automated Tests

  • test added

Manual Tests

  • a common case will be ivars in initialize with RBS typing, eg
class Foo
  def initialize
    @a = "Hello" #: String
    @b = 123
  end
end
  • Hoevering over @b should not display :String

@jesse-shopify jesse-shopify requested a review from a team as a code owner May 30, 2025 14:43
Copy link

graphite-app bot commented May 30, 2025

How to use the Graphite Merge Queue

Add the label graphite-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@jesse-shopify jesse-shopify self-assigned this May 30, 2025
@jesse-shopify jesse-shopify force-pushed the selective-comments-as-docs branch from 06a0680 to 97e4f7e Compare May 30, 2025 14:47
@jesse-shopify jesse-shopify added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels May 30, 2025
@jesse-shopify jesse-shopify force-pushed the selective-comments-as-docs branch from 97e4f7e to 1ab311b Compare May 30, 2025 19:55
@jesse-shopify jesse-shopify merged commit b911923 into main Jun 3, 2025
44 checks passed
@jesse-shopify jesse-shopify deleted the selective-comments-as-docs branch June 3, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants