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 which command has also been changed to always use the which crate
first to avoid shell specific issues (for example nushell).

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 38f29dd
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 48 deletions.
100 changes: 54 additions & 46 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,51 +78,51 @@ 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 is_wsl = SETTINGS.get::<CmdLineSettings>().wsl;

// The which crate won't work in WSL, a shell always needs to be started
// Otherwise use that first to avoid shell specific problems
if !is_wsl {
if let Ok(path) = which::which(bin) {
return path.into_os_string().into_string().ok()
}
}

// Platform command failed, fallback to which crate
if let Ok(path) = which::which(bin) {
path.into_os_string().into_string().ok()
} else {
None
// But if that does not work, try the shell anyway
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());
}
}
None
}

#[cfg(target_os = "macos")]
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 38f29dd

Please sign in to comment.