Skip to content

Index constants with RBS indexer#615

Merged
alexcrocha merged 1 commit intomainfrom
02-27-index_constants_with_rbs_indexer
Mar 3, 2026
Merged

Index constants with RBS indexer#615
alexcrocha merged 1 commit intomainfrom
02-27-index_constants_with_rbs_indexer

Conversation

@alexcrocha
Copy link
Contributor

@alexcrocha alexcrocha commented Feb 27, 2026

One more step towards #87.

This PR adds visit_constant_node so top-level, qualified, and class-nested constants are indexed.

Also extracts collect_comments to deduplicate comment collection across visitors, as suggested in #606 (comment), and moves assert_def_comments_eq! to the shared test macros in local_graph_test.

Copy link
Contributor Author

alexcrocha commented Feb 27, 2026

@alexcrocha alexcrocha self-assigned this Feb 27, 2026
@alexcrocha alexcrocha added the enhancement New feature or request label Feb 27, 2026
@alexcrocha alexcrocha marked this pull request as ready for review February 27, 2026 22:57
@alexcrocha alexcrocha requested a review from a team as a code owner February 27, 2026 22:57
@alexcrocha alexcrocha force-pushed the 02-27-index_constants_with_rbs_indexer branch from 10959f1 to ed6eba1 Compare February 27, 2026 23:34
}
}

fn collect_comments(comment_node: Option<CommentNode>) -> Vec<Comment> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should split the comment into lines. A RBS comment has multi line contents, but a Rubydex comment only has single-line content.

I think I have an implementation for this. Will share it later.

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean splitting into multiple lines at the RBS parser level?

The challenge with splitting during indexing is when the comment's content includes a line break, which ends up requiring more careful parsing. E.g.:

# something \n

Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Especially considering time zone differences, I would vote to move forward with this stack of PRs and we can revisit comment collection in a new one (to avoid blocking each other).

}
}

fn collect_comments(comment_node: Option<CommentNode>) -> Vec<Comment> {
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean splitting into multiple lines at the RBS parser level?

The challenge with splitting during indexing is when the comment's content includes a line break, which ends up requiring more careful parsing. E.g.:

# something \n

This adds visit_constant_node so top-level, qualified, and
class-nested constants are indexed.

Also extracts collect_comments to deduplicate comment collection
across the class, module, and constant visitors.
@alexcrocha alexcrocha force-pushed the 02-27-index_constants_with_rbs_indexer branch from ed6eba1 to 60b8df1 Compare March 3, 2026 03:45
@alexcrocha
Copy link
Contributor Author

Especially considering time zone differences, I would vote to move forward with this stack of PRs and we can revisit comment collection in a new one (to avoid blocking each other).

I'll move ahead and merge the stack. We can fix forward as needed

@alexcrocha alexcrocha merged commit 4af9419 into main Mar 3, 2026
33 checks passed
@alexcrocha alexcrocha deleted the 02-27-index_constants_with_rbs_indexer branch March 3, 2026 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants