Skip to content

Skip node locating when the document is not parsed #343

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
Oct 25, 2022
Merged

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Oct 25, 2022

Motivation

Although DocumentHighlight#run skips #visit when the document is not parsed, DocumentHighlight#initialize doesn't skip node locating.

This means that DocumentHighlight will fail to initialize and cause error when the source code has syntax error.

Fixes #326

Implementation

Check if the document is parsed before trying to find the target node from document.tree.

Automated Tests

Added a test to make sure the DocumentHighlight request can still be initialized with syntax-incorrect source code.

Manual Tests

  1. Create a file that has syntax error, like
    def foo(bar
      bar
    end
  2. Switch to OUTPUT tab
  3. Move cursor on foo to trigger documentHighlight requests
    • On main, you'd see
      Received response 'textDocument/documentHighlight - (40)' in 2ms. Request failed: #<NoMethodError: undefined method `child_nodes' for nil:NilClass
      
              matched = parent.child_nodes.compact.bsearch do |child|
                              ^^^^^^^^^^^^> (-32603).
      
    • On this branch, there should be no error

Verified

This commit was signed with the committer’s verified signature.
st0012 Stan Lo
Although `DocumentHighlight#run` skips `#visit` when the document is not parsed,
`DocumentHighlight#initialize` doesn't skip node locating.

This means that `DocumentHighlight` will fail to initialize and cause error when
the source code has syntax error.
@st0012 st0012 added the bug Something isn't working label Oct 25, 2022
@st0012 st0012 requested a review from a team as a code owner October 25, 2022 11:40
@st0012 st0012 self-assigned this Oct 25, 2022
@st0012 st0012 merged commit fa2d9fb into main Oct 25, 2022
@st0012 st0012 deleted the st0012-fix-#326 branch October 25, 2022 13:47
@shopify-shipit shopify-shipit bot temporarily deployed to production October 25, 2022 17:34 Inactive
vinistock pushed a commit that referenced this pull request Feb 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…0.16.16

Bump esbuild from 0.16.13 to 0.16.16
klaaspieter pushed a commit to klaaspieter/ruby-lsp that referenced this pull request Feb 25, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document highlight breaking
3 participants