Skip to content

Commit

Permalink
Improve nvim detection:
Browse files Browse the repository at this point in the history
Don't rely on the shell specific `exists", instead run `nvim -v`.
Additionally, if there's unexpected output, for example if your shell is
configured wrongly to output something when run in non-interactive mode,
it will tell you so, instead of failing with very strange errors later.

The `neovim-bin` argument has also been changed to always require the
binary to exist, instead if falling back to `nvim` as that's probably
not what the user wants. If `nevoim-bin` contains path separators the
binary will be tried directly, otherwise `which` will be used to find
the correct executable.

The output is printed directly to stderr instead of the log, for a more
user friendly experience. Furthermore, the maybe disown call has been
moved so that the user actually has a chance to see the errors in the
console.
  • Loading branch information
fredizzimo committed Jul 26, 2023
1 parent 39af18d commit 4bb4450
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 43 deletions.
87 changes: 46 additions & 41 deletions src/bridge/command.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use std::{
env,
path::Path,
env, eprintln,
process::{Command as StdCommand, Stdio},
};

use log::{debug, error, warn};
use log::debug;
use tokio::process::Command as TokioCommand;

use crate::{cmd_line::CmdLineSettings, settings::*};
Expand Down Expand Up @@ -33,22 +32,31 @@ fn set_windows_creation_flags(cmd: &mut TokioCommand) {

fn build_nvim_cmd() -> TokioCommand {
if let Some(path) = SETTINGS.get::<CmdLineSettings>().neovim_bin {
if platform_exists(&path) {
return build_nvim_cmd_with_args(&path);
} else {
warn!("NEOVIM_BIN is invalid falling back to first bin in PATH");
// if neovim_bin contains a path separator, then try to launch it directly
// otherwise use which to find the fully path
if path.contains('/') || path.contains('\\') {
if neovim_ok(&path) {
return build_nvim_cmd_with_args(&path);
}
} else if let Some(path) = platform_which(&path) {
if neovim_ok(&path) {
return build_nvim_cmd_with_args(&path);
}
}
}
if let Some(path) = platform_which("nvim") {
build_nvim_cmd_with_args(&path)
} else {
error!("nvim not found!");

eprintln!("ERROR: NEOVIM_BIN='{}' is was not found.", path);
std::process::exit(1);
} else if let Some(path) = platform_which("nvim") {
if neovim_ok(&path) {
return build_nvim_cmd_with_args(&path);
}
}
eprintln!("ERROR: nvim not found!");
std::process::exit(1);
}

// Creates a shell command if needed on this platform (wsl or macos)
fn create_platform_shell_command(command: &str, args: &[&str]) -> Option<StdCommand> {
fn create_platform_shell_command(command: &str, args: &[&str]) -> StdCommand {
if cfg!(target_os = "windows") && SETTINGS.get::<CmdLineSettings>().wsl {
let mut result = StdCommand::new("wsl");
result.args(["$SHELL", "-lc"]);
Expand All @@ -59,7 +67,7 @@ fn create_platform_shell_command(command: &str, args: &[&str]) -> Option<StdComm
winapi::um::winbase::CREATE_NO_WINDOW,
);

Some(result)
result
} else if cfg!(target_os = "macos") {
let shell = env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_string());
let mut result = StdCommand::new(shell);
Expand All @@ -70,42 +78,39 @@ fn create_platform_shell_command(command: &str, args: &[&str]) -> Option<StdComm
result.arg("-c");
result.arg(format!("{} {}", command, args.join(" ")));

Some(result)
result
} else {
None
// On Linux, just run the command directly
let mut result = StdCommand::new(command);
result.args(args);
result
}
}

#[cfg(target_os = "windows")]
fn platform_exists(bin: &str) -> bool {
// exists command is only on windows
if let Some(mut exists_command) = create_platform_shell_command("exists", &["-x", bin]) {
if let Ok(output) = exists_command.output() {
output.status.success()
} else {
error!("Exists failed");
std::process::exit(1);
fn neovim_ok(bin: &str) -> bool {
let mut cmd = create_platform_shell_command(bin, &["-v"]);
if let Ok(output) = cmd.output() {
if output.status.success() {
let stdout = String::from_utf8(output.stdout).unwrap();
if !(stdout.starts_with("NVIM v") && output.stderr.is_empty()) {
eprintln!("ERROR: Unexpected output from:\n\t{bin} -v.\n\tCheck that your shell don't output anyhting extra when running:\n\t$SHELL -lc '{bin} -v'");
std::process::exit(1);
}
return true;
}
} else {
Path::new(&bin).exists()
}
}

#[cfg(not(target_os = "windows"))]
fn platform_exists(bin: &str) -> bool {
Path::new(&bin).exists()
false
}

fn platform_which(bin: &str) -> Option<String> {
if let Some(mut which_command) = create_platform_shell_command("which", &[bin]) {
debug!("Running which command: {:?}", which_command);
if let Ok(output) = which_command.output() {
if output.status.success() {
let nvim_path = String::from_utf8(output.stdout).unwrap();
return Some(nvim_path.trim().to_owned());
} else {
return None;
}
let mut which_command = create_platform_shell_command("which", &[bin]);
debug!("Running which command: {:?}", which_command);
if let Ok(output) = which_command.output() {
if output.status.success() {
let nvim_path = String::from_utf8(output.stdout).unwrap();
return Some(nvim_path.trim().to_owned());
} else {
return None;
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ fn protected_main() {

trace!("Neovide version: {}", crate_version!());

maybe_disown();

#[cfg(target_os = "windows")]
windows_fix_dpi();

Expand All @@ -176,6 +174,8 @@ fn protected_main() {
start_bridge();
start_editor();
create_window();

maybe_disown();
}

#[cfg(not(test))]
Expand Down

0 comments on commit 4bb4450

Please sign in to comment.