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

Consider --preview flag for server subcommand #12208

Merged
merged 1 commit into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions crates/ruff/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,9 +494,20 @@ pub struct FormatCommand {

#[derive(Copy, Clone, Debug, clap::Parser)]
pub struct ServerCommand {
/// Enable preview mode; required for regular operation
#[arg(long)]
pub(crate) preview: bool,
/// Enable preview mode. Use `--no-preview` to disable.
///
/// This enables unstable server features and turns on the preview mode for the linter
/// and the formatter.
#[arg(long, overrides_with("no_preview"))]
preview: bool,
#[clap(long, overrides_with("preview"), hide = true)]
no_preview: bool,
}

impl ServerCommand {
pub(crate) fn resolve_preview(self) -> Option<bool> {
resolve_bool_arg(self.preview, self.no_preview)
}
}

#[derive(Debug, Clone, Copy, clap::ValueEnum)]
Expand Down
12 changes: 5 additions & 7 deletions crates/ruff/src/commands/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@ use crate::ExitStatus;
use anyhow::Result;
use ruff_server::Server;

pub(crate) fn run_server(preview: bool, worker_threads: NonZeroUsize) -> Result<ExitStatus> {
if !preview {
tracing::error!("--preview needs to be provided as a command line argument while the server is still unstable.\nFor example: `ruff server --preview`");
return Ok(ExitStatus::Error);
}

let server = Server::new(worker_threads)?;
pub(crate) fn run_server(
worker_threads: NonZeroUsize,
preview: Option<bool>,
) -> Result<ExitStatus> {
let server = Server::new(worker_threads, preview)?;

server.run().map(|()| ExitStatus::Success)
}
4 changes: 1 addition & 3 deletions crates/ruff/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,13 @@ fn format(args: FormatCommand, global_options: GlobalConfigArgs) -> Result<ExitS
}

fn server(args: ServerCommand) -> Result<ExitStatus> {
let ServerCommand { preview } = args;

let four = NonZeroUsize::new(4).unwrap();

// by default, we set the number of worker threads to `num_cpus`, with a maximum of 4.
let worker_threads = std::thread::available_parallelism()
.unwrap_or(four)
.max(four);
commands::server::run_server(preview, worker_threads)
commands::server::run_server(worker_threads, args.resolve_preview())
}

pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<ExitStatus> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"settings": [
{
"cwd": "/Users/test/projects/first",
"workspace": "file:///Users/test/projects/first"
},
{
"cwd": "/Users/test/projects/second",
"workspace": "file:///Users/test/projects/second"
}
],
"globalSettings": {
"cwd": "/",
"workspace": "/"
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"settings": [
{
"experimentalServer": true,
"nativeServer": "on",
"cwd": "/Users/test/projects/pandas",
"workspace": "file:///Users/test/projects/pandas",
"path": [],
Expand All @@ -21,20 +21,19 @@
"lint": {
"enable": true,
"run": "onType",
"args": [
"--preview"
]
"args": []
},
"format": {
"args": []
},
"enable": true,
"organizeImports": true,
"fixAll": true,
"showNotifications": "off"
"showNotifications": "off",
"showSyntaxErrors": true
},
{
"experimentalServer": true,
"nativeServer": "on",
"cwd": "/Users/test/projects/scipy",
"workspace": "file:///Users/test/projects/scipy",
"path": [],
Expand All @@ -55,21 +54,20 @@
"enable": true,
"preview": false,
"run": "onType",
"args": [
"--preview"
]
"args": []
},
"format": {
"args": []
},
"enable": true,
"organizeImports": true,
"fixAll": true,
"showNotifications": "off"
"showNotifications": "off",
"showSyntaxErrors": true
}
],
"globalSettings": {
"experimentalServer": true,
"nativeServer": "on",
"cwd": "/",
"workspace": "/",
"path": [],
Expand All @@ -89,16 +87,15 @@
"preview": true,
"select": ["F", "I"],
"run": "onType",
"args": [
"--preview"
]
"args": []
},
"format": {
"args": []
},
"enable": true,
"organizeImports": true,
"fixAll": false,
"showNotifications": "off"
"showNotifications": "off",
"showSyntaxErrors": true
}
}
14 changes: 9 additions & 5 deletions crates/ruff_server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub struct Server {
}

impl Server {
pub fn new(worker_threads: NonZeroUsize) -> crate::Result<Self> {
pub fn new(worker_threads: NonZeroUsize, preview: Option<bool>) -> crate::Result<Self> {
let connection = ConnectionInitializer::stdio();

let (id, init_params) = connection.initialize_start()?;
Expand All @@ -70,14 +70,18 @@ impl Server {

crate::message::init_messenger(connection.make_sender());

let AllSettings {
global_settings,
mut workspace_settings,
} = AllSettings::from_value(
let mut all_settings = AllSettings::from_value(
init_params
.initialization_options
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::default())),
);
if let Some(preview) = preview {
all_settings.set_preview(preview);
}
let AllSettings {
global_settings,
mut workspace_settings,
} = all_settings;

crate::trace::init_tracing(
connection.make_sender(),
Expand Down
92 changes: 88 additions & 4 deletions crates/ruff_server/src/session/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,20 @@ pub struct ClientSettings {
pub(crate) tracing: TracingSettings,
}

impl ClientSettings {
/// Update the preview flag for the linter and the formatter with the given value.
pub(crate) fn set_preview(&mut self, preview: bool) {
match self.lint.as_mut() {
None => self.lint = Some(LintOptions::default().with_preview(preview)),
Some(lint) => lint.set_preview(preview),
}
match self.format.as_mut() {
None => self.format = Some(FormatOptions::default().with_preview(preview)),
Some(format) => format.set_preview(preview),
}
}
}

/// Settings needed to initialize tracing. These will only be
/// read from the global configuration.
#[derive(Debug, Deserialize, Default)]
Expand All @@ -107,7 +121,7 @@ struct WorkspaceSettings {
workspace: Url,
}

#[derive(Debug, Deserialize)]
#[derive(Debug, Default, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
struct LintOptions {
Expand All @@ -118,13 +132,35 @@ struct LintOptions {
ignore: Option<Vec<String>>,
}

impl LintOptions {
fn with_preview(mut self, preview: bool) -> LintOptions {
self.preview = Some(preview);
self
}

fn set_preview(&mut self, preview: bool) {
self.preview = Some(preview);
}
}

#[derive(Debug, Default, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
struct FormatOptions {
preview: Option<bool>,
}

impl FormatOptions {
fn with_preview(mut self, preview: bool) -> FormatOptions {
self.preview = Some(preview);
self
}

fn set_preview(&mut self, preview: bool) {
self.preview = Some(preview);
}
}

#[derive(Debug, Default, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -159,6 +195,7 @@ enum InitializationOptions {
}

/// Built from the initialization options provided by the client.
#[derive(Debug)]
pub(crate) struct AllSettings {
pub(crate) global_settings: ClientSettings,
/// If this is `None`, the client only passed in global settings.
Expand All @@ -179,6 +216,16 @@ impl AllSettings {
)
}

/// Update the preview flag for both the global and all workspace settings.
pub(crate) fn set_preview(&mut self, preview: bool) {
self.global_settings.set_preview(preview);
if let Some(workspace_settings) = self.workspace_settings.as_mut() {
for settings in workspace_settings.values_mut() {
settings.set_preview(preview);
}
}
}

fn from_init_options(options: InitializationOptions) -> Self {
let (global_settings, workspace_settings) = match options {
InitializationOptions::GlobalOnly { settings } => (settings, None),
Expand Down Expand Up @@ -393,6 +440,11 @@ mod tests {
const EMPTY_INIT_OPTIONS_FIXTURE: &str =
include_str!("../../resources/test/fixtures/settings/empty.json");

// This fixture contains multiple workspaces with empty initialization options. It only sets
// the `cwd` and the `workspace` value.
const EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE: &str =
include_str!("../../resources/test/fixtures/settings/empty_multiple_workspace.json");

fn deserialize_fixture<T: DeserializeOwned>(content: &str) -> T {
serde_json::from_str(content).expect("test fixture JSON should deserialize")
}
Expand Down Expand Up @@ -456,7 +508,9 @@ mod tests {
exclude: None,
line_length: None,
configuration_preference: None,
show_syntax_errors: None,
show_syntax_errors: Some(
true,
),
tracing: TracingSettings {
log_level: None,
log_file: None,
Expand Down Expand Up @@ -509,7 +563,9 @@ mod tests {
exclude: None,
line_length: None,
configuration_preference: None,
show_syntax_errors: None,
show_syntax_errors: Some(
true,
),
tracing: TracingSettings {
log_level: None,
log_file: None,
Expand Down Expand Up @@ -575,7 +631,9 @@ mod tests {
exclude: None,
line_length: None,
configuration_preference: None,
show_syntax_errors: None,
show_syntax_errors: Some(
true,
),
tracing: TracingSettings {
log_level: None,
log_file: None,
Expand Down Expand Up @@ -771,4 +829,30 @@ mod tests {

assert_eq!(options, InitializationOptions::default());
}

fn assert_preview_client_settings(settings: &ClientSettings, preview: bool) {
assert_eq!(settings.lint.as_ref().unwrap().preview.unwrap(), preview);
assert_eq!(settings.format.as_ref().unwrap().preview.unwrap(), preview);
}

fn assert_preview_all_settings(all_settings: &AllSettings, preview: bool) {
assert_preview_client_settings(&all_settings.global_settings, preview);
if let Some(workspace_settings) = all_settings.workspace_settings.as_ref() {
for settings in workspace_settings.values() {
assert_preview_client_settings(settings, preview);
}
}
}

#[test]
fn test_preview_flag() {
let options = deserialize_fixture(EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE);
let mut all_settings = AllSettings::from_init_options(options);

all_settings.set_preview(false);
assert_preview_all_settings(&all_settings, false);

all_settings.set_preview(true);
assert_preview_all_settings(&all_settings, true);
}
}
Loading