tools
tools copied to clipboard
feat(editors/vscode): Add `requiresConfiguration` option
Summary
This PR introduces a new rome.requiresConfiguration option that disables the linter and formatter if a workspace doesn't contain a rome.json configuration.
The addition of the option is motivated by #3506 where users want an easy way to disable Rome for all projects not using Rome.
The new option doesn't fully disable Rome but only disables the linter and formatter (operations performed automatically on save). This has the benefit that other actions like sorting imports or refactors remain available.
The implementation for the linter and formatter differ:
- Formatter: I changed the extension to not register the formatting capabilities if the
rome.jsonfile is missing and therequiresConfigurationoption is set. This allows the user to e.g. use the default VS Code formatter instead and avoids the situation where a user triggers "Format Document" and nothing happens. - Linter: Returns an empty list of diagnostics from
pull_diagnostic
Test Plan
I tested that:
- Formatting and linting is working if the option is false
- Formatting and linting is working if the option is
trueand arome.jsonfile is present - Formatting and linting is disabled if the option is
trueand the project has norome.jsonfile
Deploy Preview for docs-rometools canceled.
| Name | Link |
|---|---|
| Latest commit | 0a8ec9a3c10e274aefc042e17f60dcf23adddbcc |
| Latest deploy log | https://app.netlify.com/sites/docs-rometools/deploys/63a5822ddc54d200086b21e1 |
It looks like the merge conflict with #4044 runs a bit deeper than a simple text-based merge, I can look into properly rebasing this branch as this new feature could be used to improve the way the language server handles configuration loading errors (by also disabling linting and formatting when the configuration file fails to load)
I've rebased the branch on main, and included a small refactor on the Session to allow the feature introduced in this PR to continue working after #4044 has been merged and the content of the configuration file is no longer kept in memory in the session: the code responsible for loading the configuration now writes to a configuration_status field on the Session that reflects whether a configuration was loaded correctly, has failed to parse or did not exist. This status is used in the new is_linting_and_formatting_disabled method to disable linting and formatting if the configuration failed to load, or if no configuration file exists and the requiresConfiguration option has been set.
Does the PR require updating some documentation around the repository/website?
I've rebased the branch on
main, and included a small refactor on theSessionto allow the feature introduced in this PR to continue working after #4044 has been merged and the content of the configuration file is no longer kept in memory in the session: the code responsible for loading the configuration now writes to aconfiguration_statusfield on theSessionthat reflects whether a configuration was loaded correctly, has failed to parse or did not exist. This status is used in the newis_linting_and_formatting_disabledmethod to disable linting and formatting if the configuration failed to load, or if no configuration file exists and therequiresConfigurationoption has been set.
Thanks @leops Let me clarify the description of the configuration before merging and mention the minimal required rome version
Does the PR require updating some documentation around the repository/website?
I can add a paragraph to the editor README
Patch
I'm no longer able to commit to this PR. Here are the documentation changes:Subject: [PATCH] Improve documentation
---
Index: editors/vscode/README.md
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/editors/vscode/README.md b/editors/vscode/README.md
--- a/editors/vscode/README.md (revision 75a66c74e698c9464e706fe9b82edaebb8c06e21)
+++ b/editors/vscode/README.md (revision 0d244a5b684f92f7687cbe450516fbae7df0771d)
@@ -88,6 +88,10 @@
Enables Rome to handle renames in the workspace (experimental).
+### `rome.requireConfiguration`
+
+Disables formatting, linting, and syntax errors for projects without a `rome.json` file. Requires Rome 12 or newer.
+
## Known limitations
### Configuration per sub-directory
@@ -100,10 +104,6 @@
### Disable Rome for workspaces without a `rome.json` configuration
-Rome's VS Code extension is active for every workspace regardless if the workspace contains a `rome.json` configuration ([issue 3506](https://github.com/rome/tools/issues/3506)). That may be surprising to you if you use other extensions like ESLint where the extension is disabled if the configuration is missing. This behavior is intentional because it's Rome's philosophy that the configuration should be optional.
+You can set Rome's [`rome.requireConfiguration`](#romerequireconfiguration) setting to `true` to disable Rome's formatter, linter, and syntax errors for projects without a `rome.json` file.
-You can work around this limitation by [disabling Rome per workspace](https://code.visualstudio.com/docs/editor/extension-marketplace#_disable-an-extension):
-- Open the _Extensions_ panel
-- Search for Rome
-- Right-click on _Rome_ and select _Disable (Workspace)_
Index: editors/vscode/package.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/editors/vscode/package.json b/editors/vscode/package.json
--- a/editors/vscode/package.json (revision 75a66c74e698c9464e706fe9b82edaebb8c06e21)
+++ b/editors/vscode/package.json (revision 0d244a5b684f92f7687cbe450516fbae7df0771d)
@@ -107,7 +107,7 @@
"rome.requireConfiguration": {
"type": "boolean",
"default": false,
- "markdownDescription": "Require a Rome configuration file to enable formatting and linting."
+ "markdownDescription": "Require a Rome configuration file to enable syntax errors, formatting and linting. Requires Rome 12 or newer."
}
}
},
I found another issue that must be solved before landing. Toggling the setting multiple times results in the rome formatter being registered multiple times.

A very collaborative PR :). Looks good to me. @leops you can hit the merge button when you're ready.