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.

Sign up for GitHub

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

Jump to bottom

Fix crash when cwd is deleted #7185

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Fix crash when cwd is deleted #7185

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae7ecf8
feat: store current dir internally
yo-main Jun 23, 2023
70183d3
feat: display appropriate error msg when cwd does not exist
yo-main Jul 10, 2023
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 3 additions & 5 deletions helix-core/src/path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub fn get_normalized_path(path: &Path) -> PathBuf {
pub fn get_canonicalized_path(path: &Path) -> std::io::Result<PathBuf> {
let path = expand_tilde(path);
let path = if path.is_relative() {
std::env::current_dir().map(|current_dir| current_dir.join(path))?
helix_loader::current_working_dir().join(path)
} else {
path
};
Expand All @@ -99,9 +99,7 @@ pub fn get_canonicalized_path(path: &Path) -> std::io::Result<PathBuf> {
pub fn get_relative_path(path: &Path) -> PathBuf {
let path = PathBuf::from(path);
let path = if path.is_absolute() {
let cwdir = std::env::current_dir()
.map(|path| get_normalized_path(&path))
.expect("couldn't determine current directory");
let cwdir = get_normalized_path(&helix_loader::current_working_dir());
get_normalized_path(&path)
.strip_prefix(cwdir)
.map(PathBuf::from)
Expand Down Expand Up @@ -142,7 +140,7 @@ pub fn get_relative_path(path: &Path) -> PathBuf {
/// ```
///
pub fn get_truncated_path<P: AsRef<Path>>(path: P) -> PathBuf {
let cwd = std::env::current_dir().unwrap_or_default();
let cwd = helix_loader::current_working_dir();
let path = path
.as_ref()
.strip_prefix(cwd)
Expand Down
1 change: 1 addition & 0 deletions helix-loader/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ which = "4.4"
cc = { version = "1" }
threadpool = { version = "1.0" }
tempfile = "3.6.0"
dunce = "1.0.4"

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
libloading = "0.8"
44 changes: 42 additions & 2 deletions helix-loader/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,44 @@ pub mod grammar;

use etcetera::base_strategy::{choose_base_strategy, BaseStrategy};
use std::path::{Path, PathBuf};
use std::sync::RwLock;

pub const VERSION_AND_GIT_HASH: &str = env!("VERSION_AND_GIT_HASH");

static CWD: RwLock<Option<PathBuf>> = RwLock::new(None);

static RUNTIME_DIRS: once_cell::sync::Lazy<Vec<PathBuf>> =
once_cell::sync::Lazy::new(prioritize_runtime_dirs);

static CONFIG_FILE: once_cell::sync::OnceCell<PathBuf> = once_cell::sync::OnceCell::new();

static LOG_FILE: once_cell::sync::OnceCell<PathBuf> = once_cell::sync::OnceCell::new();

// Get the current working directory.
// This information is managed internally as the call to std::env::current_dir
// might fail if the cwd has been deleted.
pub fn current_working_dir() -> PathBuf {
if let Some(path) = &*CWD.read().unwrap() {
return path.clone();
}

let path = std::env::current_dir()
.and_then(dunce::canonicalize)
.expect("Couldn't determine current working directory");
let mut cwd = CWD.write().unwrap();
*cwd = Some(path.clone());

path
}

pub fn set_current_working_dir(path: PathBuf) -> std::io::Result<()> {
let path = dunce::canonicalize(path)?;
std::env::set_current_dir(path.clone())?;
let mut cwd = CWD.write().unwrap();
*cwd = Some(path);
Ok(())
}

pub fn initialize_config_file(specified_file: Option<PathBuf>) {
let config_file = specified_file.unwrap_or_else(default_config_file);
ensure_parent_dir(&config_file);
Expand Down Expand Up @@ -217,7 +245,7 @@ pub fn merge_toml_values(left: toml::Value, right: toml::Value, merge_depth: usi
/// If no workspace was found returns (CWD, true).
/// Otherwise (workspace, false) is returned
pub fn find_workspace() -> (PathBuf, bool) {
let current_dir = std::env::current_dir().expect("unable to determine current directory");
let current_dir = current_working_dir();
for ancestor in current_dir.ancestors() {
if ancestor.join(".git").exists() || ancestor.join(".helix").exists() {
return (ancestor.to_owned(), false);
Expand All @@ -243,9 +271,21 @@ fn ensure_parent_dir(path: &Path) {
mod merge_toml_tests {
use std::str;

use super::merge_toml_values;
use super::{current_working_dir, merge_toml_values, set_current_working_dir};
use toml::Value;

#[test]
fn current_dir_is_set() {
let new_path = dunce::canonicalize(std::env::temp_dir()).unwrap();
let cwd = current_working_dir();
assert_ne!(cwd, new_path);

set_current_working_dir(new_path.clone()).expect("Couldn't set new path");

let cwd = current_working_dir();
assert_eq!(cwd, new_path);
}

#[test]
fn language_toml_map_merges() {
const USER: &str = r#"
Expand Down
2 changes: 1 addition & 1 deletion helix-lsp/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ pub fn find_lsp_workspace(
let mut file = if file.is_absolute() {
file.to_path_buf()
} else {
let current_dir = std::env::current_dir().expect("unable to determine current directory");
let current_dir = helix_loader::current_working_dir();
current_dir.join(file)
};
file = path::get_normalized_path(&file);
Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/application.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl Application {
} else if !args.files.is_empty() {
let first = &args.files[0].0; // we know it's not empty
if first.is_dir() {
std::env::set_current_dir(first).context("set current dir")?;
helix_loader::set_current_working_dir(first.clone())?;
editor.new_file(Action::VerticalSplit);
let picker = ui::file_picker(".".into(), &config.load().editor);
compositor.push(Box::new(overlaid(picker)));
Expand Down
5 changes: 2 additions & 3 deletions helix-term/src/commands.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the two changes here we will probably want some feedback:

if !cwd.exists() {
    editor.set_error("Current working directory does not exist");
    return;
}

since the file_picker_in_current_directory won't show any files and global_search won't be able to find any matches if the cwd doesn't exist.

Original file line number Diff line number Diff line change
Expand Up @@ -2075,8 +2075,7 @@ fn global_search(cx: &mut Context) {
.binary_detection(BinaryDetection::quit(b'\x00'))
.build();

let search_root = std::env::current_dir()
.expect("Global search error: Failed to get current dir");
let search_root = helix_loader::current_working_dir();
let dedup_symlinks = file_picker_config.deduplicate_links;
let absolute_root = search_root
.canonicalize()
Expand Down Expand Up @@ -2514,7 +2513,7 @@ fn file_picker_in_current_buffer_directory(cx: &mut Context) {
cx.push_layer(Box::new(overlaid(picker)));
}
fn file_picker_in_current_directory(cx: &mut Context) {
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("./"));
let cwd = helix_loader::current_working_dir();
let picker = ui::file_picker(cwd, &cx.editor.config());
cx.push_layer(Box::new(overlaid(picker)));
}
Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/commands/dap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ pub fn dap_start_impl(
}
}

args.insert("cwd", to_value(std::env::current_dir().unwrap())?);
args.insert("cwd", to_value(helix_loader::current_working_dir())?);

let args = to_value(args).unwrap();

Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/commands/lsp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ fn goto_impl(
locations: Vec<lsp::Location>,
offset_encoding: OffsetEncoding,
) {
let cwdir = std::env::current_dir().unwrap_or_default();
let cwdir = helix_loader::current_working_dir();

match locations.as_slice() {
[location] => {
Expand Down
9 changes: 3 additions & 6 deletions helix-term/src/commands/typed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,14 +1093,11 @@ fn change_current_directory(
.as_ref(),
);

if let Err(e) = std::env::set_current_dir(dir) {
bail!("Couldn't change the current working directory: {}", e);
}
helix_loader::set_current_working_dir(dir)?;

let cwd = std::env::current_dir().context("Couldn't get the new working directory")?;
cx.editor.set_status(format!(
"Current working directory is now {}",
cwd.display()
helix_loader::current_working_dir().display()
));
Ok(())
}
Expand All @@ -1114,7 +1111,7 @@ fn show_current_directory(
return Ok(());
}

let cwd = std::env::current_dir().context("Couldn't get the new working directory")?;
let cwd = helix_loader::current_working_dir();
cx.editor
.set_status(format!("Current working directory is {}", cwd.display()));
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/ui/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ pub mod completers {
match path.parent() {
Some(path) if !path.as_os_str().is_empty() => path.to_path_buf(),
// Path::new("h")'s parent is Some("")...
_ => std::env::current_dir().expect("couldn't determine current directory"),
_ => helix_loader::current_working_dir(),
}
};

Expand Down
Loading