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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

show symlink destination #374

Merged
merged 1 commit into from Jun 8, 2020
Merged

show symlink destination #374

merged 1 commit into from Jun 8, 2020

Conversation

wedens
Copy link
Contributor

@wedens wedens commented Jun 6, 2020

Closes #196.

Shows symlink destination at the bottom, at the right of permissions/size/time.

If it's a symlink to a symlink, it'll resolve only the first one, which I think makes sense.

I've also tried adding "target" option to info, but fairly long info strings are not being displayed at all. Even when there is a lot of space.

PS: this is my first ever code in Go 馃檪

@gokcehan
Copy link
Owner

gokcehan commented Jun 7, 2020

@wedens Thank you very much for the patch. This has been missing for quite a while and I'm glad it is finally getting implemented.

I have tried the patch and it seems to be working great. A minor nitpick in the code is that you seem to ignore the error returned by os.Readlink which I usually try to avoid. Do you know how does os.Readlink fail? If it's a rare error maybe just add an early return with the error (e.g. if err != nil { return files, err }) so at least it is logged. Does that sound ok?

@wedens
Copy link
Contributor Author

wedens commented Jun 8, 2020

Do you know how does os.Readlink fail?

It seems to fail on invalid path or if it's not a symlink. Those conditions are already checked before os.Readlink call.

If it's a rare error maybe just add an early return with the error (e.g. if err != nil { return files, err }) so at least it is logged. Does that sound ok?

I've added those changes. I'm not a Go developer, so I wasn't sure what's the best practice for unlikely errors ;)

@gokcehan
Copy link
Owner

gokcehan commented Jun 8, 2020

@wedens Seems good to me, thanks again.

@gokcehan gokcehan merged commit 8933a3e into gokcehan:master Jun 8, 2020
@gokcehan gokcehan mentioned this pull request Aug 19, 2020
gokcehan added a commit that referenced this pull request Aug 19, 2020
Provessor added a commit to Provessor/lf that referenced this pull request Sep 1, 2020
Fix colour construction issue

This also has a test to mitigate it in the future

Remove `colormode` option

The original issue it was trying to solve is no longer present with
tcell (it being a holdover from `color256` on termbox) so it is not
needed.

retire gitter channel in favor of irc/matrix

Export options as environment variables (gokcehan#448)

* Export options as environment variables

Any options from gOpts are available via lf_OPTION environment
variables. For now it works only on booleans, integers and strings (no
array support)

* Do not export some of the options

* Add support for arrays and fix numbers

* Fix comments

* Replace 1 and 0 with true and false

* Export hidden,reverse,dirfirst and sortby options

* Fix comments

* Little fix

* Simplify boolean conversion

log readlink errors instead of fail

Related gokcehan#447 and gokcehan#374
gokcehan pushed a commit that referenced this pull request Sep 1, 2020
Fix colour construction issue

This also has a test to mitigate it in the future

Remove `colormode` option

The original issue it was trying to solve is no longer present with
tcell (it being a holdover from `color256` on termbox) so it is not
needed.

retire gitter channel in favor of irc/matrix

Export options as environment variables (#448)

* Export options as environment variables

Any options from gOpts are available via lf_OPTION environment
variables. For now it works only on booleans, integers and strings (no
array support)

* Do not export some of the options

* Add support for arrays and fix numbers

* Fix comments

* Replace 1 and 0 with true and false

* Export hidden,reverse,dirfirst and sortby options

* Fix comments

* Little fix

* Simplify boolean conversion

log readlink errors instead of fail

Related #447 and #374
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Target file/directory is not shown for symbolic links
2 participants