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

chore(fmt/psl): update find at position w/ spans #4957

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Druue
Copy link
Contributor

@Druue Druue commented Jul 17, 2024

Updated pest grammar for empty argument to include the whitespaces up until whatever comes next. This means we have a wider span to use for empty arguments and enables us to check for cursors that are where the ""value"" would be but there is no actual value yet.

EnumValuePosition was manually calculating attribute positions, this has been removed - it now defers to the AttributePosition implementation.

Updating the positions in find_at_position to also include spans

Extracted out reference_locations_for_target to separate module in prep for other lsp commands (e.g. rename)

… until whatever comes next. This means we have a wider span to use for empty arguments and enables us to check for cursors that are where the ""value"" would be but there is no actual value yet.

EnumValuePosition was manually calculating attribute positions, this has been removed - it now defers to the AttributePosition implementation.

Updating the positions in `find_at_position` to also include spans

Extracted out reference_locations_for_target to separate module in prep for other lsp commands (e.g. rename)
@Druue Druue added the kind/tech A technical change. label Jul 17, 2024
@Druue Druue added this to the 5.18.0 milestone Jul 17, 2024
@Druue Druue self-assigned this Jul 17, 2024
@Druue Druue requested a review from a team as a code owner July 17, 2024 19:57
@Druue Druue requested review from SevInf and removed request for a team July 17, 2024 19:57
@@ -114,7 +114,7 @@ field_attribute = { "@" ~ path ~ arguments_list? }
// ######################################
arguments_list = { "(" ~ (argument ~ ("," ~ argument)*)? ~ trailing_comma? ~ ")" }
argument = _{ named_argument | empty_argument | expression }
empty_argument = { identifier ~ ":" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows us an empty argument span that includes all the whitespace until the next , or ).

}

impl<'ast> AttributePosition<'ast> {
pub(crate) fn new(attr: &'ast ast::Attribute, position: usize) -> Self {
let mut spans: Vec<(Option<&str>, ast::Span)> = attr
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer necessary due to the addition of a whitespace inclusive span for empty args. We can just use contains now.

Copy link

codspeed-hq bot commented Jul 17, 2024

CodSpeed Performance Report

Merging #4957 will not alter performance

Comparing chore/find_at_position_spans (17c65d2) with main (a2d1c42)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Jul 17, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.058MiB 2.057MiB 474.000B
Postgres (gzip) 820.710KiB 820.453KiB 264.000B
Mysql 2.028MiB 2.027MiB 474.000B
Mysql (gzip) 808.268KiB 808.035KiB 239.000B
Sqlite 1.920MiB 1.919MiB 474.000B
Sqlite (gzip) 766.460KiB 766.277KiB 188.000B

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/tech A technical change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant