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

Fix expansion of ~ #284

Merged
merged 6 commits into from
Jun 18, 2021
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
2 changes: 2 additions & 0 deletions helix-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ pub fn cache_dir() -> std::path::PathBuf {
path
}

pub use etcetera::home_dir;

use etcetera::base_strategy::{choose_base_strategy, BaseStrategy};

pub use ropey::{Rope, RopeSlice};
Expand Down
6 changes: 3 additions & 3 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1070,8 +1070,8 @@ mod cmd {
.filter(|doc| doc.is_modified())
.map(|doc| {
doc.relative_path()
.and_then(|path| path.to_str())
.unwrap_or("[scratch]")
.map(|path| path.to_string_lossy().to_string())
Copy link
Member

Choose a reason for hiding this comment

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

No need for to_string() here, it's already a Cow I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunatelly:

error[E0515]: cannot return value referencing function parameter `path`
    --> helix-term/src/commands.rs:1073:33
     |
1073 |                     .map(|path| path.to_string_lossy())
     |                                 ----^^^^^^^^^^^^^^^^^^
     |                                 |
     |                                 returns a value referencing data owned by the current function
     |                                 `path` is borrowed here

Copy link
Contributor Author

@vv9k vv9k Jun 18, 2021

Choose a reason for hiding this comment

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

It is a consequence of changing the return type of relative_path to be owned PathBuf not &Path but I'm not sure I can return a &Path while still folding home directory into tilde.

.unwrap_or_else(|| "[scratch]".into())
})
.collect();
if !modified.is_empty() {
Expand Down Expand Up @@ -1369,7 +1369,7 @@ pub fn buffer_picker(cx: &mut Context) {
cx.editor
.documents
.iter()
.map(|(id, doc)| (id, doc.relative_path().map(Path::to_path_buf)))
.map(|(id, doc)| (id, doc.relative_path()))
.collect(),
move |(id, path): &(DocumentId, Option<PathBuf>)| {
// format_fn
Expand Down
18 changes: 14 additions & 4 deletions helix-term/src/ui/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,11 @@ pub mod completers {
use ignore::WalkBuilder;
use std::path::{Path, PathBuf};

let path = Path::new(input);
let is_tilde = input.starts_with('~') && input.len() == 1;
let path = helix_view::document::expand_tilde(Path::new(input));

let (dir, file_name) = if input.ends_with('/') {
(path.into(), None)
(path, None)
} else {
let file_name = path
.file_name()
Expand All @@ -152,7 +153,16 @@ pub mod completers {
let is_dir = entry.file_type().map_or(false, |entry| entry.is_dir());

let path = entry.path();
let mut path = path.strip_prefix(&dir).unwrap_or(path).to_path_buf();
let mut path = if is_tilde {
// if it's a single tilde an absolute path is displayed so that when `TAB` is pressed on
// one of the directories the tilde will be replaced with a valid path not with a relative
// home directory name.
// ~ -> <TAB> -> /home/user
// ~/ -> <TAB> -> ~/first_entry
path.to_path_buf()
} else {
path.strip_prefix(&dir).unwrap_or(path).to_path_buf()
};

if is_dir {
path.push("");
Expand Down Expand Up @@ -182,7 +192,7 @@ pub mod completers {
})
.collect();

let range = ((input.len() - file_name.len())..);
let range = ((input.len().saturating_sub(file_name.len()))..);

matches.sort_unstable_by_key(|(_file, score)| Reverse(*score));
files = matches
Expand Down
61 changes: 52 additions & 9 deletions helix-view/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,36 @@ where
}
}

/// Expands tilde `~` into users home directory if avilable, otherwise returns the path
/// unchanged. The tilde will only be expanded when present as the first component of the path
/// and only slash follows it.
pub fn expand_tilde(path: &Path) -> PathBuf {
let mut components = path.components().peekable();
if let Some(Component::Normal(c)) = components.peek() {
if c == &"~" {
if let Ok(home) = helix_core::home_dir() {
// it's ok to unwrap, the path starts with `~`
return home.join(path.strip_prefix("~").unwrap());
}
}
}

path.to_path_buf()
}

/// Replaces users home directory from `path` with tilde `~` if the directory
/// is available, otherwise returns the path unchanged.
pub fn fold_home_dir(path: &Path) -> PathBuf {
if let Ok(home) = helix_core::home_dir() {
if path.starts_with(&home) {
// it's ok to unwrap, the path starts with home dir
return PathBuf::from("~").join(path.strip_prefix(&home).unwrap());
}
}

path.to_path_buf()
}

/// Normalize a path, removing things like `.` and `..`.
///
/// CAUTION: This does not resolve symlinks (unlike
Expand All @@ -112,6 +142,7 @@ where
/// needs to improve on.
/// Copied from cargo: https://github.com/rust-lang/cargo/blob/070e459c2d8b79c5b2ac5218064e7603329c92ae/crates/cargo-util/src/paths.rs#L81
pub fn normalize_path(path: &Path) -> PathBuf {
let path = expand_tilde(path);
let mut components = path.components().peekable();
let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() {
components.next();
Expand All @@ -138,12 +169,17 @@ pub fn normalize_path(path: &Path) -> PathBuf {
ret
}

// Returns the canonical, absolute form of a path with all intermediate components normalized.
//
// This function is used instead of `std::fs::canonicalize` because we don't want to verify
// here if the path exists, just normalize it's components.
/// Returns the canonical, absolute form of a path with all intermediate components normalized.
///
/// This function is used instead of `std::fs::canonicalize` because we don't want to verify
/// here if the path exists, just normalize it's components.
pub fn canonicalize_path(path: &Path) -> std::io::Result<PathBuf> {
std::env::current_dir().map(|current_dir| normalize_path(&current_dir.join(path)))
Copy link
Member

Choose a reason for hiding this comment

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

You still want this because paths need to be converted to absolute. Not taking current_dir() into account will consider myfile.c and /full/path/to/myfile.c as two separate files.

Copy link
Member

Choose a reason for hiding this comment

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

let normalized = normalize_path(path);
if normalized.is_absolute() {
Ok(normalized)
} else {
std::env::current_dir().map(|current_dir| current_dir.join(normalized))
}
}

use helix_lsp::lsp;
Expand Down Expand Up @@ -684,12 +720,19 @@ impl Document {
&self.selections[&view_id]
}

pub fn relative_path(&self) -> Option<&Path> {
pub fn relative_path(&self) -> Option<PathBuf> {
let cwdir = std::env::current_dir().expect("couldn't determine current directory");

self.path
.as_ref()
.map(|path| path.strip_prefix(cwdir).unwrap_or(path))
self.path.as_ref().map(|path| {
let path = fold_home_dir(path);
if path.is_relative() {
path
} else {
path.strip_prefix(cwdir)
.map(|p| p.to_path_buf())
.unwrap_or(path)
}
})
}

// pub fn slice<R>(&self, range: R) -> RopeSlice where R: RangeBounds {
Expand Down