feat(clap-complete/unstable-dynamic): add '~' to HOME dir completion support #5998
Conversation
eatradish
force-pushed
the
home-dir-symbol
branch
from
2ec365a to
9b172a4
Compare
…support if input starts with '~' will complete to `/home/USER`
eatradish
force-pushed
the
home-dir-symbol
branch
from
9b172a4 to
42c4b84
Compare
| #[cfg(unix)] | ||
| if current.starts_with("~") { |
There was a problem hiding this comment.
This is marked as unix-only but the dep is always added
However, should we support this on Windows?
There was a problem hiding this comment.
I'm not sure if this feature should be supported on Windows, as it does not seem to be used on Windows.
| #[cfg(unix)] | ||
| if current.starts_with("~") { |
There was a problem hiding this comment.
Technically, there are other variants of ~ expansion. Should we support that?
There was a problem hiding this comment.
I'm not sure about what they are, can you explain?
| @@ -283,7 +295,7 @@ impl ValueCompleter for PathCompleter { | |||
| } | |||
|
|
|||
| pub(crate) fn complete_path( | |||
| value_os: &OsStr, | |||
| value_os: &str, | |||
There was a problem hiding this comment.
Why are we changing this to be more restrictive?
There was a problem hiding this comment.
There doesn't seem to be a way for me to get &OsStr directly from String, maybe I should? (The reason for the change to String is that the replace_range method is only available for String)
let mut new_current = OsString::new();
new_current.write_str(¤t).ok();
let mut candidates = complete_path(&new_current, current_dir, filter);
| let Some(current) = current.to_str() else { | ||
| return vec![]; | ||
| }; |
There was a problem hiding this comment.
Maybe I should have used let mut current = current.to_string_lossy().to_string(); ?
| @@ -40,6 +40,7 @@ is_executable = { version = "1.0.1", optional = true } | |||
| shlex = { version = "1.3.0", optional = true } | |||
| completest = { version = "0.4.2", optional = true } | |||
| completest-pty = { version = "0.5.5", optional = true } | |||
| home = { version = "0.5.0", optional = true } | |||
There was a problem hiding this comment.
With std::env::home_dir being undeprecated, we should be using this through a polyfll. I'll need to go create one before this can move forward.
There was a problem hiding this comment.
(if it wasn't for that, I'd be hesitant to add this out of question of whether the extra dep is worth it)
There was a problem hiding this comment.
Perhaps we could copy that function from std? https://github.com/rust-lang/rust/blob/2682b88c526d493edeb2d3f2df358f44db69b73f/library/std/src/sys/unix/os.rs#L595
if input starts with '~' will complete to
/home/USER