Skip to content

Migrate Selection Ranges to YARP - #895

Merged
andyw8 merged 1 commit into
yarpfrom
andyw8/migrate-selection-ranges-to-yarp
Aug 30, 2023
Merged

Migrate Selection Ranges to YARP#895
andyw8 merged 1 commit into
yarpfrom
andyw8/migrate-selection-ranges-to-yarp

Conversation

@andyw8

@andyw8andyw8 commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

Partially addresses #449

Paired on with @vinistock for parts.

Note: Some fixtures intentionally changed to be closer to how YARP represents the code.

@github-actions

github-actionsBot commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

github-actionsBot commented Aug 17, 2023

Copy link
Copy Markdown
Contributor
Benchmark results in seconds (slowest at top)
textDocument/completion average: 0.256368 std_dev: 0.008868
textDocument/diagnostic average: 0.059678 std_dev: 0.013802
textDocument/definition average: 0.006414 std_dev: 0.003423
textDocument/selectionRange average: 0.004716 std_dev: 0.000745
textDocument/documentHighlight average: 0.001998 std_dev: 0.000438
codeAction/resolve average: 0.001704 std_dev: 0.00028
textDocument/hover average: 0.001663 std_dev: 0.000439
textDocument/inlayHint average: 0.001474 std_dev: 0.000236
textDocument/codeLens average: 0.001234 std_dev: 0.000311
textDocument/documentSymbol average: 0.001232 std_dev: 0.000309
textDocument/documentLink average: 0.001223 std_dev: 0.000263
textDocument/semanticTokens/full average: 0.001204 std_dev: 0.000283
textDocument/onTypeFormatting average: 0.001189 std_dev: 0.00028
textDocument/codeAction average: 0.001147 std_dev: 0.000168
textDocument/semanticTokens/range average: 0.001141 std_dev: 0.000206
textDocument/formatting average: 0.001112 std_dev: 0.000317
textDocument/foldingRange average: 0.001094 std_dev: 0.000228
================================================================================
Comparison with main branch:
textDocument/semanticTokens/full faster by 65.624 %
textDocument/semanticTokens/range faster by 46.616 %
textDocument/documentSymbol faster by 63.921 %
textDocument/foldingRange faster by 66.699 %
textDocument/formatting unchanged
textDocument/diagnostic unchanged
textDocument/documentLink faster by 64.665 %
textDocument/inlayHint faster by 21.502 %
textDocument/selectionRange faster by 16.955 %
textDocument/documentHighlight faster by 42.947 %
textDocument/hover unchanged
textDocument/codeAction unchanged
textDocument/onTypeFormatting unchanged
codeAction/resolve unchanged
textDocument/completion unchanged
textDocument/codeLens faster by 65.106 %
textDocument/definition unchanged
================================================================================
Missing benchmarks:
RubyLsp::Requests::ShowSyntaxTree

@andyw8
andyw8force-pushed the andyw8/migrate-selection-ranges-to-yarp branch 2 times, most recently from 823821b to 61031a1CompareAugust 18, 2023 14:52
@andyw8andyw8 mentioned this pull request Aug 20, 2023
29 tasks
@andyw8
andyw8force-pushed the andyw8/migrate-selection-ranges-to-yarp branch 2 times, most recently from 49439ce to 0406ca7CompareAugust 30, 2023 17:10
},
"end": {
"line": 0,
"character": 6

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

end columns are off by 1 due to a YARP issue which is being addressed separately.

@andyw8
andyw8force-pushed the andyw8/migrate-selection-ranges-to-yarp branch from 0406ca7 to 9cd1df7CompareAugust 30, 2023 17:23
).returns(Support::SelectionRange)
end
def create_selection_range(location, parent = nil)
def create_selection_range(location, parent)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I removed the unnecessary nil default for parent since we always pass something.

@andyw8
andyw8force-pushed the andyw8/migrate-selection-ranges-to-yarp branch from 9cd1df7 to 1efa776CompareAugust 30, 2023 17:30
"character": 0
}
},
"parent": {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

If it helps for reviewing, I can add a note to explain each 'substantial' change in the expectations.

@andyw8
andyw8 marked this pull request as ready for review August 30, 2023 17:32
@andyw8
andyw8 requested a review from a team as a code ownerAugust 30, 2023 17:32
@andyw8
andyw8 requested review from st0012 and vinistockAugust 30, 2023 17:32
Comment threadlib/ruby_lsp/requests/selection_ranges.rb Outdated
@andyw8
andyw8force-pushed the andyw8/migrate-selection-ranges-to-yarp branch from 3a5d6eb to 8be1661CompareAugust 30, 2023 18:01
@andyw8
andyw8 merged commit 0c22413 into yarpAug 30, 2023
@andyw8
andyw8 deleted the andyw8/migrate-selection-ranges-to-yarp branch August 30, 2023 18:08
vinistock pushed a commit that referenced this pull request Feb 28, 2024
Avoid adding yjit flag since ruby 3.3
Sign up for freeto 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.

2 participants

@andyw8@vinistock