Skip to content

Use symlink target for stat styling & icons#1644

Merged
gokcehan merged 6 commits into
gokcehan:masterfrom
lorentzforces:ln-target-colors
Apr 7, 2024
Merged

Use symlink target for stat styling & icons#1644
gokcehan merged 6 commits into
gokcehan:masterfrom
lorentzforces:ln-target-colors

Conversation

@lorentzforces

@lorentzforces lorentzforces commented Mar 17, 2024

Copy link
Copy Markdown
Contributor

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.

@joelim-work joelim-work left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment thread colors.go Outdated
Comment thread colors.go Outdated
Comment thread colors.go Outdated
Comment thread doc.md Outdated
@lorentzforces

lorentzforces commented Mar 19, 2024

Copy link
Copy Markdown
Contributor Author

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

@joelim-work joelim-work left a comment

Copy link
Copy Markdown
Collaborator

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
@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