ls: fix symlink target coloring to match target file type #9006
Conversation
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
src/uu/ls/src/ls.rs
Outdated
| config: &Config, | ||
| command_line: bool, | ||
| ) -> Self { | ||
| Self::new_with_dereference_override(p_buf, dir_entry, file_name, config, command_line, None) |
There was a problem hiding this comment.
what is the point of keeping this function ?
There was a problem hiding this comment.
Sorry, it was from an earlier patchset.
src/uu/ls/src/ls.rs
Outdated
| if command_line { | ||
| if let Ok(md) = p_buf.metadata() { | ||
| md.is_dir() | ||
| let must_dereference = |
There was a problem hiding this comment.
maybe move this block into a new function to simplify the code
|
GNU testsuite comparison: |
CodSpeed Performance ReportMerging #9006 will not alter performanceComparing Summary
Footnotes
|
|
GNU testsuite comparison: |
| target_symlink: Option<&PathData>, | ||
| wrap: bool, | ||
| ) -> OsString { | ||
| color_name_with_dangling_hint(name, path, style_manager, target_symlink, false, wrap) |
There was a problem hiding this comment.
sorry for the latency but i am not sure to see the point of a function with only one line.
color_name_with_dangling_hint should be merged back here
|
I believe this one can be closed, there have been other PR's that have addressed the issue of symlink target coloring and when testing locally on dirs, files, executables and broken symlinks they all matched for me locally |
|
Sorry, I completely misunderstood the purpose of this PR. I see now that its specifically about the symlink chains, not just about symlinks. I did some testing locally to see if symlink coloring was implemented and assumed that it already was after testing. I did some more research into this problem and I think the solution is much simpler: #10274 There was simply a fs::metadata call that should have been a canonicalize call to get the end target instead. By taking the current approach its also missing:
On an unrelated not it looks like the upstream is missing coloring for .tar and .jpg/.mp4 files compared to gnu |
3 participants
fixes #8934