Skip to content

Conversation

@vinistock
Copy link
Member

Motivation

We're still getting some errors while users are actively editing the Gemfile. I couldn't figure out exactly what triggers some of them, but I figured it would be worth to make the implementation more explicit that we only handle string literals.

Implementation

Started returning early if the first argument is not a string literal. And then if inside that literal the first part is not a string content.

Automated Tests

Added a new test.

@vinistock vinistock added the bugfix This PR will fix an existing bug label Jun 20, 2023
@vinistock vinistock added this to the 2023-Q2 milestone Jun 20, 2023
@vinistock vinistock self-assigned this Jun 20, 2023
@vinistock vinistock requested a review from a team as a code owner June 20, 2023 20:40
@vinistock vinistock requested review from Morriar and andyw8 June 20, 2023 20:40
@github-actions
Copy link
Contributor

Benchmark results in seconds (slowest at top)

          textDocument/completion average: 0.297889 std_dev: 0.00575
          textDocument/diagnostic average: 0.048597 std_dev: 0.010534
      textDocument/selectionRange average: 0.003947 std_dev: 0.000566
   textDocument/documentHighlight average: 0.002068 std_dev: 0.000173
 textDocument/semanticTokens/full average: 0.001967 std_dev: 0.000206
        textDocument/documentLink average: 0.00196 std_dev: 0.000152
      textDocument/documentSymbol average: 0.00195 std_dev: 0.000111
            textDocument/codeLens average: 0.001938 std_dev: 0.000149
        textDocument/foldingRange average: 0.001891 std_dev: 0.000117
textDocument/semanticTokens/range average: 0.001008 std_dev: 4.8e-05
               codeAction/resolve average: 0.000742 std_dev: 7.9e-05
           textDocument/inlayHint average: 0.000692 std_dev: 5.1e-05
               textDocument/hover average: 0.000691 std_dev: 7.6e-05
    textDocument/onTypeFormatting average: 0.00013 std_dev: 6.4e-05
          textDocument/formatting average: 7.8e-05 std_dev: 0.000328
          textDocument/codeAction average: 4.4e-05 std_dev: 3.3e-05


================================================================================
Comparison with main branch:

 textDocument/semanticTokens/full unchanged
textDocument/semanticTokens/range unchanged
      textDocument/documentSymbol unchanged
        textDocument/foldingRange unchanged
          textDocument/formatting unchanged
          textDocument/diagnostic unchanged
        textDocument/documentLink unchanged
           textDocument/inlayHint unchanged
      textDocument/selectionRange unchanged
   textDocument/documentHighlight unchanged
               textDocument/hover unchanged
          textDocument/codeAction unchanged
    textDocument/onTypeFormatting unchanged
               codeAction/resolve unchanged
          textDocument/completion unchanged
            textDocument/codeLens unchanged

@vinistock vinistock merged commit dcd6e22 into main Jun 20, 2023
@vinistock vinistock deleted the vs/make_gem_remote_more_robust branch June 20, 2023 21:08
@snutij
Copy link
Contributor

snutij commented Jun 21, 2023

Hi @vinistock, I'm sorry about this feature which needed 2 bug fixes. I was focused on "reading" a Gemfile, without thinking about the users who can edit it or edge-case like that!

I'm curious about the We're still getting some errors do you monitor errors/usages somewhere? 🤔

Thanks for both fixes!

@vinistock
Copy link
Member Author

Hey @snutij! Don't worry, it's hard to predict what intermediate states files can be in.

The Ruby LSP extension allows connecting to a private metrics extension (docs). We collect metrics internally from our devs to find crashes.

@shopify-shipit shopify-shipit bot temporarily deployed to production June 23, 2023 15:44 Inactive
vinistock pushed a commit that referenced this pull request Feb 28, 2024
…re-7.22.15

Bump @babel/core from 7.22.11 to 7.22.15
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants