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

Add support for cmd.exe (experimental) #567

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

mataha
Copy link
Contributor

@mataha mataha commented May 10, 2023

See #218.

This PR adds experimental support for cmd.exe using the following Batch techniques:

  • DOSKEY macros
  • for /f with disappearing carets
  • "smart" percent characters

Most of the work was done in a0c6535 - % characters have different meanings depending on the context in which the code is interpreted (command-line vs script), thus the beginning of the template file introduces a macro used to escape them regardless of the situation - even in for statements.

While working on this I had to fix path normalization on Windows in bc947f4, i.e. patching drive letter prefixes to always be uppercase (as that's how the underlying API expects them to be, even though cmd.exe will happily return an unnormalized component from volume path environment variable). A similar problem was encountered in rust-analyzer, which made me provide a fix there. To offset the potential overhead I've bumped dunce to 1.0.4 in 2fdde1c, though most of the changes there are related to licensing and packaging.

To test this, zoxide init cmd is piped into cmd.exe - anything on stderr means failure, e.g.:

')' is not recognized as an internal or external command,
operable program or batch file.

shell.nix Outdated Show resolved Hide resolved
src/shell.rs Show resolved Hide resolved
@@ -94,6 +98,22 @@ mod tests {
}

#[apply(opts)]
#[cfg(windows)]
fn cmd_cmd(cmd: Option<&str>, hook: InitHook, echo: bool, resolve_symlinks: bool) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't run as #[cfg(feature = "nix-dev")] is in effect. Should I pull it to a separate module?

@mataha
Copy link
Contributor Author

mataha commented Jun 6, 2023

It is ready.

@mataha mataha marked this pull request as ready for review June 6, 2023 21:37
@ajeetdsouza
Copy link
Owner

This is awesome! I'm learning Batch as I review this, so forgive the intermittent comments.

README.md Outdated Show resolved Hide resolved
templates/cmd.txt Outdated Show resolved Hide resolved
mataha added a commit to mataha/zoxide that referenced this pull request Aug 16, 2023
When it comes to resolving paths on Windows, even though the underlying
API expects drive letter prefixes to be uppercase, some sources (e.g.
environment variables like `=C:`) won't normalize components, instead
returning the value as-is. While this wouldn't be a problem normally as
NTFS is case-insensitive on Windows, this introduces duplicates in the
database when adding new entries via `zoxide add`:

```batchfile prompt
> zoxide query --list
D:\
d:\
D:\coding
d:\coding
D:\coding\.cloned
d:\coding\.cloned
```

This is a cherry-pick from ajeetdsouza#567; see also rust-lang/rust-analyzer#14683.

Signed-off-by: mataha <[email protected]>
mataha added a commit to mataha/zoxide that referenced this pull request Dec 15, 2023
When it comes to resolving paths on Windows, even though the underlying
API expects drive letter prefixes to be uppercase, some sources (e.g.
environment variables like `=C:`) won't normalize components, instead
returning the value as-is. While this wouldn't be a problem normally as
NTFS is case-insensitive on Windows, this introduces duplicates in the
database when adding new entries via `zoxide add`:

```batchfile prompt
> zoxide query --list
D:\
d:\
D:\coding
d:\coding
D:\coding\.cloned
d:\coding\.cloned
```

This is a cherry-pick from ajeetdsouza#567; see also rust-lang/rust-analyzer#14683.

Signed-off-by: mataha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants