Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_lsp): don't throw error for ignored files in code actions #4443

Merged
merged 3 commits into from
May 8, 2023

Conversation

ematipico
Copy link
Contributor

Summary

Closes #4434

I also took the opportunity to refactor how "features" are requested from a workspace. So far, requesting a feature was done via .supports_feature, and it was possible only to ask for a "feature" per file.

I refactored the code; now it's possible to request multiple features. The result of the function also provides some APIs to ask if a file supports a single "feature".

The concept of supports_feature has been renamed to file_features.

Test Plan

I created a new test for the bug.

The refactor should not have any regression, so the existing tests should all pass.

Changelog

  • The PR requires a changelog line

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented May 8, 2023

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit 9c953a7
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/64590223313011000878ae7d

@github-actions github-actions bot added A-CLI Area: CLI A-Formatter Area: formatter A-Linter Area: linter A-LSP Area: language server protocol A-Parser Area: parser A-Project Area: project configuration and loading labels May 8, 2023
Comment on lines +199 to +201
let mut file_features = FileFeaturesResult::new()
.with_capabilities(&capabilities)
.with_settings(&settings);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Features are now built from capabilities and settings, simplifying the old logic.

@github-actions
Copy link

github-actions bot commented May 8, 2023

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 48863 48863 0
Passed 47796 47796 0
Failed 1067 1067 0
Panics 0 0 0
Coverage 97.82% 97.82% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6212 6212 0
Passed 1763 1763 0
Failed 4449 4449 0
Panics 0 0 0
Coverage 28.38% 28.38% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 573 573 0
Failed 66 66 0
Panics 0 0 0
Coverage 89.67% 89.67% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17224 17224 0
Passed 13127 13127 0
Failed 4097 4097 0
Panics 0 0 0
Coverage 76.21% 76.21% 0.00%

@ematipico ematipico merged commit 1d86eec into main May 8, 2023
@ematipico ematipico deleted the fix/ignored-files branch May 8, 2023 15:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-CLI Area: CLI A-Formatter Area: formatter A-Linter Area: linter A-LSP Area: language server protocol A-Parser Area: parser A-Project Area: project configuration and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Enabling organizeImports and ignoring ["node_modules"] folder breaks rome
1 participant