-
-
Notifications
You must be signed in to change notification settings - Fork 257
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
feat: Added auto
, always
, never
modifiers to --hyperlink
argument (V2)
#1059
base: main
Are you sure you want to change the base?
Conversation
I can merge shell completions updates into my branch, but I don't see how this PR resolves what was blocking on mine. |
For conflicts, you have rebasing. But that's not blocking. The issue we had was that hyperlink OSCs were not taken into account when calculating width and that end up messing with the I just rebased and force-pushed on my end. Works fine apparently. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code and functionality on my system looks good ... Gotta check this failing BSD test and conflict with main, though.
Ok, I rebased main into this branch and resolved the conflicts. |
I rebased main again which added a fix for the FreeBSD CI, should run through now ... also ran a short benchmark, doesn't appear to introduce any performance penalty, so again LGTM, thanks. |
21e5316
to
bc3edc2
Compare
… can Co-authored-by: Milo Moisson <milomoisson@gmail.com> Co-authored-by: Sandro-Alessio Gierens <sandro@gierens.de>
Co-authored-by: Milo Moisson <milomoisson@gmail.com> Co-authored-by: Sandro-Alessio Gierens <sandro@gierens.de>
Co-authored-by: Sandro-Alessio Gierens <sandro@gierens.de>
Co-authored-by: Milo Moisson <milomoisson@gmail.com> Co-authored-by: Sandro-Alessio Gierens <sandro@gierens.de>
The merge-base changed after approval.
This is an ideological continuation of @mrnossiom's work in #746 (as I've learned already after I started coding the current PR myself) with added shell completions that don't have merge conflicts with the
main
branch.Description
Add a
WHEN
modifier to the--hyperlink
argument. This matches thels
behavior and the--color
argument behavior.I think
ls
is doing it right because you might want to enable--hyperlink
in a shell alias but still get plain text when piping into some other program without control chars in the way. Setting--hyperlink
in a shell alias is useful for terminal integration (e.g. Kitty).The public API is compatible since
--hyperlink
defaults toauto
and doesn't break. However, it changes the behavior, with no value the option worked likealways
but now defaults toauto
. Even though I considerauto
the right default, maybe it could be changed toalways
to keep the old behavior.