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

Commit

Permalink
address PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
leops committed Nov 10, 2022
1 parent 3d5820e commit d2ebdc5
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 34 deletions.
4 changes: 2 additions & 2 deletions crates/rome_cli/src/commands/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ pub(crate) fn stop(mut session: CliSession) -> Result<(), Termination> {
pub(crate) fn run_server(mut session: CliSession) -> Result<(), Termination> {
setup_tracing_subscriber();

let is_oneshot = session.args.contains("--oneshot");
let stop_on_disconnect = session.args.contains("--stop-on-disconnect");

let rt = Runtime::new()?;
let factory = ServerFactory::new(is_oneshot);
let factory = ServerFactory::new(stop_on_disconnect);
let cancellation = factory.cancellation();
let span = debug_span!("Running Server", pid = std::process::id());

Expand Down
10 changes: 5 additions & 5 deletions crates/rome_cli/src/service/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ async fn try_connect() -> io::Result<UnixStream> {
}

/// Spawn the daemon server process in the background
fn spawn_daemon(is_oneshot: bool) -> io::Result<Child> {
fn spawn_daemon(stop_on_disconnect: bool) -> io::Result<Child> {
let binary = env::current_exe()?;

let mut cmd = Command::new(binary);
cmd.arg("__run_server");

if is_oneshot {
cmd.arg("--oneshot");
if stop_on_disconnect {
cmd.arg("--stop-on-disconnect");
}

// Create a new session for the process and make it the leader, this will
Expand Down Expand Up @@ -106,7 +106,7 @@ pub(crate) async fn open_socket() -> io::Result<Option<(OwnedReadHalf, OwnedWrit
///
/// Returns false if the daemon process was already running or true if it had
/// to be started
pub(crate) async fn ensure_daemon(is_oneshot: bool) -> io::Result<bool> {
pub(crate) async fn ensure_daemon(stop_on_disconnect: bool) -> io::Result<bool> {
let mut current_child: Option<Child> = None;
let mut last_error = None;

Expand Down Expand Up @@ -144,7 +144,7 @@ pub(crate) async fn ensure_daemon(is_oneshot: bool) -> io::Result<bool> {
} else {
// Spawn the daemon process and wait a few milliseconds for
// it to become ready then retry the connection
current_child = Some(spawn_daemon(is_oneshot)?);
current_child = Some(spawn_daemon(stop_on_disconnect)?);
time::sleep(Duration::from_millis(50)).await;
}
}
Expand Down
10 changes: 5 additions & 5 deletions crates/rome_cli/src/service/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ async fn try_connect() -> io::Result<NamedPipeClient> {
const CREATE_NEW_PROCESS_GROUP: u32 = 0x00000200;

/// Spawn the daemon server process in the background
fn spawn_daemon(is_oneshot: bool) -> io::Result<()> {
fn spawn_daemon(stop_on_disconnect: bool) -> io::Result<()> {
let binary = env::current_exe()?;

let mut cmd = Command::new(binary);
cmd.arg("__run_server");

if is_oneshot {
cmd.arg("--oneshot");
if stop_on_disconnect {
cmd.arg("--stop-on-disconnect");
}

cmd.creation_flags(CREATE_NEW_PROCESS_GROUP);
Expand Down Expand Up @@ -168,14 +168,14 @@ impl AsyncWrite for ClientWriteHalf {
///
/// Returns false if the daemon process was already running or true if it had
/// to be started
pub(crate) async fn ensure_daemon(is_oneshot: bool) -> io::Result<bool> {
pub(crate) async fn ensure_daemon(stop_on_disconnect: bool) -> io::Result<bool> {
let mut did_spawn = false;

loop {
match open_socket().await {
Ok(Some(_)) => break,
Ok(None) => {
spawn_daemon(is_oneshot)?;
spawn_daemon(stop_on_disconnect)?;
did_spawn = true;
time::sleep(Duration::from_millis(50)).await;
}
Expand Down
18 changes: 10 additions & 8 deletions crates/rome_lsp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub struct LSPServer {
sessions: Sessions,
/// If this is true the server will broadcast a shutdown signal once the
/// last client disconnected
is_oneshot: bool,
stop_on_disconnect: bool,
/// This shared flag is set to true once at least one sessions has been
/// initialized on this server instance
is_initialized: Arc<AtomicBool>,
Expand All @@ -36,13 +36,13 @@ impl LSPServer {
fn new(
session: SessionHandle,
sessions: Sessions,
is_oneshot: bool,
stop_on_disconnect: bool,
is_initialized: Arc<AtomicBool>,
) -> Self {
Self {
session,
sessions,
is_oneshot,
stop_on_disconnect,
is_initialized,
}
}
Expand Down Expand Up @@ -348,7 +348,9 @@ impl Drop for LSPServer {
let _removed = sessions.remove(&self.session.key);
debug_assert!(_removed.is_some(), "Session did not exist.");

if self.is_oneshot && sessions.is_empty() && self.is_initialized.load(Ordering::Relaxed)
if self.stop_on_disconnect
&& sessions.is_empty()
&& self.is_initialized.load(Ordering::Relaxed)
{
self.session.cancellation.notify_one();
}
Expand Down Expand Up @@ -380,7 +382,7 @@ pub struct ServerFactory {

/// If this is true the server will broadcast a shutdown signal once the
/// last client disconnected
is_oneshot: bool,
stop_on_disconnect: bool,
/// This shared flag is set to true once at least one sessions has been
/// initialized on this server instance
is_initialized: Arc<AtomicBool>,
Expand Down Expand Up @@ -421,13 +423,13 @@ macro_rules! workspace_method {
}

impl ServerFactory {
pub fn new(is_oneshot: bool) -> Self {
pub fn new(stop_on_disconnect: bool) -> Self {
Self {
cancellation: Arc::default(),
workspace: None,
sessions: Sessions::default(),
next_session_key: AtomicU64::new(0),
is_oneshot,
stop_on_disconnect,
is_initialized: Arc::default(),
}
}
Expand All @@ -451,7 +453,7 @@ impl ServerFactory {
LSPServer::new(
handle,
self.sessions.clone(),
self.is_oneshot,
self.stop_on_disconnect,
self.is_initialized.clone(),
)
});
Expand Down
14 changes: 0 additions & 14 deletions editors/vscode/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,6 @@ Rome doesn’t yet support loading the `rome.json` file from a directory other t

Rome isn't yet able to pick up the `rome.json` configuration in [multi-root workspaces](https://code.visualstudio.com/docs/editor/multi-root-workspaces) if the configuration isn't in the first root folder ([issue 3538](https://github.com/rome/tools/issues/3538)). You can work around this limitation by making the folder with the configuration the first root folder of the workspace (drag it to the top).

### The extension uses an outdated version of Rome

Make sure to restart Rome’s LSP server after updating the extension. To stop Rome’s LSP, send the `stop` command (VS will start a new LSP instance for you):

```bash
npx rome stop

# or
pnpx exec rome stop

# or
yarn run rome stop
```

### 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.
Expand Down

0 comments on commit d2ebdc5

Please sign in to comment.