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

Commit

Permalink
Revert "feat(editors/vscode): Add requiresConfiguration option"
Browse files Browse the repository at this point in the history
This reverts commit 61914d3.
  • Loading branch information
MichaReiser committed Dec 9, 2022
1 parent 61914d3 commit d84f585
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 147 deletions.
10 changes: 8 additions & 2 deletions crates/rome_lsp/src/capabilities.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use tower_lsp::lsp_types::{
CodeActionProviderCapability, ServerCapabilities, TextDocumentSyncCapability,
TextDocumentSyncKind,
CodeActionProviderCapability, DocumentOnTypeFormattingOptions, OneOf, ServerCapabilities,
TextDocumentSyncCapability, TextDocumentSyncKind,
};

/// The capabilities to send from server as part of [`InitializeResult`]
Expand All @@ -12,6 +12,12 @@ pub(crate) fn server_capabilities() -> ServerCapabilities {
TextDocumentSyncKind::INCREMENTAL,
)),
code_action_provider: Some(CodeActionProviderCapability::Simple(true)),
document_formatting_provider: Some(OneOf::Left(true)),
document_range_formatting_provider: Some(OneOf::Left(true)),
document_on_type_formatting_provider: Some(DocumentOnTypeFormattingOptions {
first_trigger_character: String::from("}"),
more_trigger_character: Some(vec![String::from("]"), String::from(")")]),
}),
rename_provider: None,
..Default::default()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,14 @@ pub struct WorkspaceSettings {

/// Enable rename capability
pub rename: Option<bool>,

/// Only run Rome if a `rome.json` configuration file exists.
pub require_configuration: Option<bool>,
}

/// The `rome.*` extension settings
#[derive(Debug)]
pub(crate) struct ExtensionSettings {
pub(crate) struct Config {
pub(crate) settings: WorkspaceSettings,
}

impl ExtensionSettings {
impl Config {
pub(crate) fn new() -> Self {
Self {
settings: WorkspaceSettings::default(),
Expand All @@ -41,8 +37,4 @@ impl ExtensionSettings {
);
Ok(())
}

pub(crate) fn requires_configuration(&self) -> bool {
self.settings.require_configuration.unwrap_or_default()
}
}
4 changes: 2 additions & 2 deletions crates/rome_lsp/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod capabilities;
mod config;
mod documents;
mod extension_settings;
mod handlers;
mod line_index;
mod requests;
Expand All @@ -9,5 +9,5 @@ mod session;
mod url_interner;
mod utils;

pub use crate::extension_settings::WorkspaceSettings;
pub use crate::config::WorkspaceSettings;
pub use crate::server::{LSPServer, ServerConnection, ServerFactory};
120 changes: 31 additions & 89 deletions crates/rome_lsp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use rome_console::markup;
use rome_fs::CONFIG_NAME;
use rome_service::workspace::{RageEntry, RageParams, RageResult};
use rome_service::{workspace, Workspace};
use serde_json::json;
use std::collections::HashMap;
use std::panic::RefUnwindSafe;
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
Expand Down Expand Up @@ -112,50 +111,45 @@ impl LSPServer {
Ok(RageResult { entries })
}

async fn register_capabilities(&self, registrations: Vec<Registration>) {
let methods = registrations
.iter()
.map(|reg| reg.method.clone())
.collect::<Vec<_>>()
.join(", ");
async fn register_capability(&self, registration: Registration) {
let method = registration.method.clone();

if let Err(e) = self.session.client.register_capability(registrations).await {
error!("Error registering {:?} capability: {}", methods, e);
if let Err(e) = self
.session
.client
.register_capability(vec![registration])
.await
{
error!("Error registering {:?} capability: {}", method, e);
}
}

async fn unregister_capabilities(&self, registrations: Vec<Unregistration>) {
let methods = registrations
.iter()
.map(|reg| reg.method.clone())
.collect::<Vec<_>>()
.join(", ");
async fn unregister_capability(&self, unregistration: Unregistration) {
let method = unregistration.method.clone();

if let Err(e) = self
.session
.client
.unregister_capability(registrations)
.unregister_capability(vec![unregistration])
.await
{
error!("Error unregistering {:?} capability: {}", methods, e);
error!("Error unregistering {:?} capability: {}", method, e);
}
}

async fn setup_capabilities(&self) {
let rename = {
let config = self.session.extension_settings.read().ok();
let config = self.session.config.read().ok();
config.and_then(|x| x.settings.rename).unwrap_or(false)
};

let mut register = Vec::new();
let mut unregister = Vec::new();

if self.session.can_register_did_change_configuration() {
register.push(Registration {
self.register_capability(Registration {
id: "workspace/didChangeConfiguration".to_string(),
method: "workspace/didChangeConfiguration".to_string(),
register_options: None,
});
})
.await;
}

let base_path = self.session.base_path();
Expand All @@ -167,75 +161,27 @@ impl LSPServer {
kind: Some(WatchKind::all()),
}],
};
register.push(Registration {
self.register_capability(Registration {
id: "workspace/didChangeWatchedFiles".to_string(),
method: "workspace/didChangeWatchedFiles".to_string(),
register_options: Some(serde_json::to_value(registration_options).unwrap()),
});
}

if self.session.is_linting_and_formatting_disabled() {
unregister.extend([
Unregistration {
id: "textDocument/formatting".to_string(),
method: "textDocument/formatting".to_string(),
},
Unregistration {
id: "textDocument/rangeFormatting".to_string(),
method: "textDocument/rangeFormatting".to_string(),
},
Unregistration {
id: "textDocument/onTypeFormatting".to_string(),
method: "textDocument/onTypeFormatting".to_string(),
},
]);
} else {
register.extend([
Registration {
id: "textDocument/formatting".to_string(),
method: "textDocument/formatting".to_string(),
register_options: Some(json!(TextDocumentRegistrationOptions {
document_selector: None
})),
},
Registration {
id: "textDocument/rangeFormatting".to_string(),
method: "textDocument/rangeFormatting".to_string(),
register_options: Some(json!(TextDocumentRegistrationOptions {
document_selector: None
})),
},
Registration {
id: "textDocument/onTypeFormatting".to_string(),
method: "textDocument/onTypeFormatting".to_string(),
register_options: Some(json!(DocumentOnTypeFormattingRegistrationOptions {
document_selector: None,
first_trigger_character: String::from("}"),
more_trigger_character: Some(vec![String::from("]"), String::from(")")]),
})),
},
]);
})
.await;
}

if rename {
register.push(Registration {
self.register_capability(Registration {
id: "textDocument/rename".to_string(),
method: "textDocument/rename".to_string(),
register_options: None,
});
})
.await;
} else {
unregister.push(Unregistration {
self.unregister_capability(Unregistration {
id: "textDocument/rename".to_string(),
method: "textDocument/rename".to_string(),
});
}

if !register.is_empty() {
self.register_capabilities(register).await;
}

if !unregister.is_empty() {
self.unregister_capabilities(unregister).await;
})
.await;
}
}
}
Expand Down Expand Up @@ -282,10 +228,8 @@ impl LanguageServer for LSPServer {

info!("Attempting to load the configuration from 'rome.json' file");

futures::join!(
self.session.update_configuration(),
self.session.fetch_extension_settings()
);
self.session.update_configuration().await;
self.session.fetch_client_configuration().await;

let msg = format!("Server initialized with PID: {}", std::process::id());
self.session
Expand All @@ -303,13 +247,11 @@ impl LanguageServer for LSPServer {
Ok(())
}

/// Called when the user changed the editor settings.
#[tracing::instrument(level = "debug", skip(self))]
async fn did_change_configuration(&self, params: DidChangeConfigurationParams) {
let _ = params;
self.session.fetch_extension_settings().await;
self.session.fetch_client_configuration().await;
self.setup_capabilities().await;
self.session.update_all_diagnostics().await;
}

#[tracing::instrument(level = "debug", skip(self))]
Expand All @@ -327,7 +269,7 @@ impl LanguageServer for LSPServer {
if let Ok(possible_rome_json) = possible_rome_json {
if possible_rome_json.display().to_string() == CONFIG_NAME {
self.session.update_configuration().await;
self.setup_capabilities().await;
self.session.fetch_client_configuration().await;
self.session.update_all_diagnostics().await;
// for now we are only interested to the configuration file,
// so it's OK to exist the loop
Expand Down Expand Up @@ -403,7 +345,7 @@ impl LanguageServer for LSPServer {
rome_diagnostics::panic::catch_unwind(move || {
let rename_enabled = self
.session
.extension_settings
.config
.read()
.ok()
.and_then(|config| config.settings.rename)
Expand Down
31 changes: 10 additions & 21 deletions crates/rome_lsp/src/session.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::config::Config;
use crate::config::CONFIGURATION_SECTION;
use crate::documents::Document;
use crate::extension_settings::ExtensionSettings;
use crate::extension_settings::CONFIGURATION_SECTION;
use crate::url_interner::UrlInterner;
use crate::utils;
use futures::stream::futures_unordered::FuturesUnordered;
Expand Down Expand Up @@ -47,8 +47,8 @@ pub(crate) struct Session {

pub(crate) client_information: Mutex<Option<ClientInformation>>,

/// The settings of the Rome extension (under the `rome` namespace)
pub(crate) extension_settings: RwLock<ExtensionSettings>,
/// the configuration of the LSP
pub(crate) config: RwLock<Config>,

pub(crate) workspace: Arc<dyn Workspace>,

Expand Down Expand Up @@ -78,7 +78,7 @@ impl Session {
let client_capabilities = RwLock::new(Default::default());
let documents = Default::default();
let url_interner = Default::default();
let config = RwLock::new(ExtensionSettings::new());
let config = RwLock::new(Config::new());
let configuration = RwLock::new(None);
let root_uri = RwLock::new(None);
Self {
Expand All @@ -89,7 +89,7 @@ impl Session {
workspace,
documents,
url_interner,
extension_settings: config,
config,
fs: DynRef::Owned(Box::new(OsFileSystem)),
configuration,
root_uri,
Expand Down Expand Up @@ -144,10 +144,7 @@ impl Session {
path: rome_path.clone(),
})?;

let diagnostics = if self.is_linting_and_formatting_disabled() {
tracing::trace!("Linting disabled because Rome configuration is missing and `requireConfiguration` is true.");
vec![]
} else if let Some(reason) = unsupported_lint.reason {
let diagnostics = if let Some(reason) = unsupported_lint.reason {
tracing::trace!("linting not supported: {reason:?}");
// Sending empty vector clears published diagnostics
vec![]
Expand Down Expand Up @@ -245,7 +242,7 @@ impl Session {
}

/// Requests "workspace/configuration" from client and updates Session config
pub(crate) async fn fetch_extension_settings(&self) {
pub(crate) async fn fetch_client_configuration(&self) {
let item = lsp_types::ConfigurationItem {
scope_uri: None,
section: Some(String::from(CONFIGURATION_SECTION)),
Expand All @@ -258,14 +255,15 @@ impl Session {
.into_iter()
.next()
.and_then(|client_configuration| {
let mut config = self.extension_settings.write().unwrap();
let mut config = self.config.write().unwrap();

config
.set_workspace_settings(client_configuration)
.map_err(|err| {
error!("Cannot set workspace settings: {}", err);
})
.ok()?;
self.update_workspace_settings();

Some(())
});
Expand Down Expand Up @@ -318,13 +316,4 @@ impl Session {
}
}
}

pub(crate) fn is_linting_and_formatting_disabled(&self) -> bool {
self.configuration.read().unwrap().is_none()
&& self
.extension_settings
.read()
.unwrap()
.requires_configuration()
}
}
13 changes: 0 additions & 13 deletions editors/vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,6 @@
"vscode": "^1.70.0",
"npm": "^8"
},
"capabilities": {
"untrustedWorkspaces": {
"supported": "limited",
"restrictedConfigurations": [
"rome.lspBin"
]
}
},
"contributes": {
"languages": [
{
Expand Down Expand Up @@ -103,11 +95,6 @@
],
"default": null,
"markdownDescription": "Enable/Disable Rome handling renames in the workspace. (Experimental)"
},
"rome.requireConfiguration": {
"type": "boolean",
"default": false,
"markdownDescription": "Require a Rome configuration file to enable formatting and linting."
}
}
},
Expand Down
Loading

0 comments on commit d84f585

Please sign in to comment.