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

feat: Added auto, always, never modifiers to --hyperlink argument (V2) #1059

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

nikelborm
Copy link

This is continuation of @mrnossiom's work in #746 (as I've learned already after I started coding current PR myself) with added shell completions and which doesn't have merge conflicts with the main branch.

Description

Add a WHEN modifier to the --hyperlink argument. This matches both ls 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 to auto and doesn't break. However, it changes the behavior, with no value the option worked like always but it now defaults to auto. Even though I consider auto to be the right default, maybe it could be changed to always to keep old meaning.

@mrnossiom
Copy link
Contributor

I can merge shell completions updates into my branch, but I don't see how this PR resolves what was blocking on mine.

@nikelborm
Copy link
Author

nikelborm commented Jul 16, 2024

I can merge shell completions updates into my branch, but I don't see how this PR resolves what was blocking on mine.

When I started doing it, I started doing it myself following already established patterns in the current project. But I started doing it on top of the latest main branch. Then I found your PR that looked similar and merged what i done with what you did and mentioned you in the commits where our changes looked similar. I didn't really tested what kind of conflicts exist on your branch, I just looked on github's banner
Screenshot from 2024-07-16 14-41-49
And since I built this PR on top of latest commit in main branch it doesn't yet have any conflicts.

@mrnossiom
Copy link
Contributor

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 --grid/G flag.

I just rebased and force-pushed on my end. Works fine apparently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

2 participants