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

Use symlink target for stat styling & icons #1644

Merged
merged 6 commits into from
Apr 7, 2024

Conversation

lorentzforces
Copy link
Contributor

@lorentzforces lorentzforces commented Mar 17, 2024

Support LS_COLORS/dircolors use of "target" for symlinks to select icons and styles for them according to the target stat. This mainly applies to directories - file name-based styles will still be based on the name of the link, not its target. This mirrors ls behavior.

This is an attempt to address issue #566 . I am by no means an experienced Go programmer, so please do give any feedback you have.

Fixes #566

Support LS_COLORS/dircolors use of "target" for symlinks to select icons
and styles for them according to the target stat. This mainly applies to
directories - file name-based styles will still be based on the name of
the link, not its target. This mirrors `ls` behavior.
Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request.

Originally the logic for applying styles was designed to be simple. However, I think supporting a special value of target for links is a good idea since it mirrors the behavior of ls, even if it makes the logic more complex. I will leave this up to @gokcehan to decide if such a feature can be included.

I have left a few comments in the meantime, these apply for both colors and icons.

colors.go Outdated Show resolved Hide resolved
colors.go Outdated Show resolved Hide resolved
colors.go Outdated Show resolved Hide resolved
doc.md Outdated Show resolved Hide resolved
@lorentzforces
Copy link
Contributor Author

lorentzforces commented Mar 19, 2024

@joelim-work Addressed your comments. I appreciate the feedback! I left changes in individual commits for now for review. Can squash on demand.

Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. I will give my approval for now, but also leave the PR open for a while just in case anyone else is interested in reviewing.

Edit: It looks like the CI build fails because of some minor incorrect formatting, please fix this.

@gokcehan gokcehan merged commit 5b10803 into gokcehan:master Apr 7, 2024
4 checks passed
@gokcehan gokcehan mentioned this pull request Apr 7, 2024
@joelim-work joelim-work added this to the r33 milestone Jun 24, 2024
@joelim-work joelim-work added the new Pull requests that add new behavior label Jun 24, 2024
@joelim-work joelim-work changed the title Use symlink target for stat styling & icons (#566) Use symlink target for stat styling & icons Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new Pull requests that add new behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Icons of symbolic links to directories
3 participants