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 the colors when completing using a relative path #12898

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Conversation

ymcx
Copy link
Contributor

@ymcx ymcx commented May 17, 2024

Description

Fixes a bug where the autocompletion menu has the wrong colors due to 'std::fs::symlink_metadata' not being able to handle relative paths. Attached below are screenshots before and after applying the commit, in which the colors are wrong when autocompleting on a path prefixed by a tilde, whereas the same directory is highlighted correctly when prefixed by a dot.

BEFORE:
1715982514020
1715982873231
AFTER:
1715982490585
1715982894748

@novaTopFlex
Copy link

My version of nushell has a grid/spreadsheet-style layout whenever I type the ls command. I am not having the colors issue, but I am having the issue where ./ and ~/ are not being treated as synonymous while in the ~ directory on my system (ls ./ results in relative paths while ls ~/ results in absolute paths).

@ymcx
Copy link
Contributor Author

ymcx commented May 19, 2024

My version of nushell has a grid/spreadsheet-style layout whenever I type the ls command. I am not having the colors issue, but I am having the issue where ./ and ~/ are not being treated as synonymous while in the ~ directory on my system (ls ./ results in relative paths while ls ~/ results in absolute paths).

I might've worded the pull request poorly and picked the wrong command to showcase the issue with. The issue is able to be reproduced by typing in any command (not just ls) and then hitting tab in order to select a file with the autocompletion menu.
I also forgot to mention that the I was able to reproduce the bug on a Windows installition using an empty config file, so it does not seem to only affect Linux/Unix systems.

@ymcx
Copy link
Contributor Author

ymcx commented May 19, 2024

I'm unsure if relative paths are supposed to work in the metadata (and symlink_metadata by proxy) function or if the current behaviour is intended and Nushell is not using the function correctly. Anyhow, you can verify for yourself that relative paths are not supported by running the following code.

fn main() -> std::io::Result<()> {
    println!("{:?}", std::fs::metadata("/home/{USER}/foo")?);
    println!("{:?}", std::fs::metadata("~/foo")?);
    Ok(())
}
> touch ~/foo
> cargo run
Metadata { file_type: FileType(FileType { mode: 33152 }), is_dir: false, is_file: true, permissions: Permissions(FilePermissions { mode: 33152 }), modified: Ok(SystemTime { tv_sec: 1716131487, tv_nsec: 847242904 }), accessed: Ok(SystemTime { tv_sec: 1716131487, tv_nsec: 847242904 }), created: Ok(SystemTime { tv_sec: 1716131487, tv_nsec: 846340977 }), .. }
Error: Os { code: 2, kind: NotFound, message: "No such file or directory" }

@sholderbach
Copy link
Member

I think for the resolution of relative paths our utmost expert is @YizhePKU who may be able to help you there and can review this PR.

@sholderbach sholderbach added completions Issues related to tab completion file-system Related to commands and core nushell behavior around the file system labels May 20, 2024
@YizhePKU
Copy link
Contributor

YizhePKU commented May 21, 2024

I think there's some confusion of terminology. ./ is a relative path. What you actually mean is that paths that contains tilde components don't work, because std::fs::symlink_metadata doesn't accept paths with tilde.

In fact, it's not just tilde -- ndots components don't work either (e.g. .../). Tilde and ndots don't work in syscalls because they're shell-specific concepts (i.e. Nushell made it up).

Instead of trying to resolve paths yourself, you should use nu_path::expand_to_real_path(), which is created for this exact purpose.

@fdncred fdncred merged commit cfe1339 into nushell:main Jun 7, 2024
13 checks passed
@fdncred
Copy link
Collaborator

fdncred commented Jun 7, 2024

Thanks

@hustcer hustcer added this to the v0.95.0 milestone Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completions Issues related to tab completion file-system Related to commands and core nushell behavior around the file system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants