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

DocumentSymbols provider should handle modules without a name #179

Merged
merged 3 commits into from
Apr 3, 2020
Merged

DocumentSymbols provider should handle modules without a name #179

merged 3 commits into from
Apr 3, 2020

Conversation

axelson
Copy link
Member

@axelson axelson commented Apr 3, 2020

The DocumentSymbols provider should be able to handle a variety of compilation errors so that it can be useful while debugging the compilation errors. The main problem was that the extract_symbol/2 clause that handled defmodule and defprotocol was overly restrictive in the pattern match that it did. So I've changed it to match all the possible AST variations (although some may still raise an exception within the clause, but this prevents bad data from propagating).

MISSING_MODULE_NAME was returned since a non-empty name is required by the Language Server Protocol.

The DocumentSymbols provider should be able to handle a variety of compilation
errors so that it can be useful while debugging the compilation errors. The main
problem was that the `extract_symbol/2` clause that handled `defmodule` and
`defprotocol` was overly restrictive in the pattern match that it did. So I've
changed it to match all the possible AST variations (although some may still
raise an exception within the clause, but this prevents bad data from
propagating).

`MISSING_MODULE_NAME` was returned since a non-empty name is required
end: %{line: 0, character: 0}
}
}
] = document_symbols
Copy link
Contributor

Choose a reason for hiding this comment

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

Either use == or drop the assert in these two. A match error would happen before the assert making it redundant

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good point. That was me being lazy. Updated! 👍

@axelson axelson merged commit fb641e4 into elixir-lsp:master Apr 3, 2020
@axelson axelson deleted the fix-document-symbol-compile-error branch April 3, 2020 21:14
Sign up for free to 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.

None yet

2 participants