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

ls -s other-directory wrong colors for directories #15085

Open
ComputerDruid opened this issue Feb 11, 2025 · 6 comments
Open

ls -s other-directory wrong colors for directories #15085

ComputerDruid opened this issue Feb 11, 2025 · 6 comments
Labels
file-system Related to commands and core nushell behavior around the file system needs-triage An issue that hasn't had any proper look pipeline-metadata Additional information passed along that is not directly attached to the values (e.g. LSCOLORS)

Comments

@ComputerDruid
Copy link

Describe the bug

when using ls -s to list the contents of other directories, the colors are wrong for subdirectories

How to reproduce

> mkdir /tmp/a/b /tmp/b/a
> touch /tmp/a/a /tmp/b/b
> cd /tmp/b
> ls -s .
> ls -s ../a

Image

Expected behavior

When running ls -s ../a I expected the directory (b) to be colored blue, and the file (b) to be colored white, but the reverse was true.

Configuration

version | transpose key value | to md --pretty

key value
version 0.102.0
major 0
minor 102
patch 0
branch
commit_hash
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.82.0 (f6e511eec 2024-10-15)
cargo_version cargo 1.82.0 (8f40fc59f 2024-08-21)
build_time 2025-02-10 15:06:05 -08:00
build_rust_channel release
allocator mimalloc
features default, sqlite, trash
installed_plugins
@ComputerDruid ComputerDruid added the needs-triage An issue that hasn't had any proper look label Feb 11, 2025
@ComputerDruid
Copy link
Author

For comparison, external ls gets the colors right

Image

@fdncred
Copy link
Collaborator

fdncred commented Feb 11, 2025

It's a known bug. ls -s doesn't maintain LS_COLORS because our LS_COLORS implementation needs to know the full path and -s removes it.

@sholderbach sholderbach added file-system Related to commands and core nushell behavior around the file system pipeline-metadata Additional information passed along that is not directly attached to the values (e.g. LSCOLORS) labels Feb 11, 2025
@ComputerDruid
Copy link
Author

Some words about why this issue is important to me:

nushell seems like a nice language that I would enjoy using as my default shell environment. Since most commands don't produce structured output by default, I figured they would need to be wrapped in functions that switch them to a structured mode by default. Because colorized output is important to me, I started trying to figure out how to make custom commands emit colorized output (without ruining the actual file paths in case I later needed to work with them programmatically). Not finding any documentation about that, I tried to look into how the ls builtin accomplished that, and was disheartened to find that ls itself doesn't do it at all, and instead defers it toward the end, leading to predictable bugs like this one. Also it doesn't suggest a way for custom commands to accomplish the same kind of goal (colorized output that doesn't mess up future consumers of said output).

So for me, this bug is important for two big reasons:

  • The colors emitted by ls -s are legitimately important to me. My brain uses the "blue color"-ness of directories to interpret them, and ls -s has the closest output to /bin/ls so that's what I would likely want to use as default in my shell.
  • Colors in general are important to me as a terminal user, and the fact that they don't seem to be integrated into nushell in a principled way makes me feel like nushell isn't ready for me to use as a daily driver

Hopefully that's helpful as extra motivation for why this bug is important. And maybe that's all already obvious to y'all and the current implementation is just a placeholder for some grand vision that I haven't seen.

Meta note: I was motivated to leave this comment because I didn't get the sense that it was viewed as an important problem and thought sharing my perspective would help. I intend it to be constructive, but would be interested in feedback if it doesn't come across as constructive

@fdncred
Copy link
Collaborator

fdncred commented Feb 19, 2025

Thanks for sharing. We're looking for motivated users to fix problems by submitting PRs as they have time. I've tried to fix this problem before and what it required was holding a String that was the file's path and a String that was used only for Display purposes. I figured that would be too heavy handed, so I never landed it. That closed PR is probably in this repo still as closed not implemented. There may be other ways to tackle it but I'm not sure what they are.

@boomshroom
Copy link

This seems like it would be impossible if the only thing nushell has to work with is the shown string that gets printed. However that isn't all that's available. ls also gives the type of the directory entry even in short mode, and displaying that and the size also requires a stat call, which also gives the mode (which itself is shown in long mode) needed to tell if something is executable, etc.

So the needed information is still there, just not within the one cell being shown. This is particularly noticible when using grid, which reads from the given table and doesn't use the built-in path formatting, so it could read the type and mode fields if it wanted to show the correct colors and icons.

@fdncred
Copy link
Collaborator

fdncred commented Feb 19, 2025

Go for it if you think you can make it work across Win, Lin, Mac. I'd love to see it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
file-system Related to commands and core nushell behavior around the file system needs-triage An issue that hasn't had any proper look pipeline-metadata Additional information passed along that is not directly attached to the values (e.g. LSCOLORS)
Projects
None yet
Development

No branches or pull requests

4 participants