Skip to content

Conversation

@Ordoviz
Copy link
Contributor

@Ordoviz Ordoviz commented Jan 3, 2026

I used to carelessly use redirects like > file.txt instead of >? file.txt assuming that fish would underline file.txt if this file already exists (just like command arguments are underlined via fish_color_valid_path), but I recently learned that redirection targets are never underlined! This PR fixes this inconsistency.

Old New
Old New

I hope this will save me from accidentally clobbering important files.

There seem to be some failing system tests, but I want to get feedback before fixing them.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages. (not applicable)
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@krobelus
Copy link
Contributor

krobelus commented Jan 4, 2026

I like the semantics and the increased consistency.
I don't think we use the combination of --bold --underline anywhere else yet;
it might look a bit too dense (ignore the underlined > obviously)

image

Maybe that's an appropriate amount of loudness for overwriting an existing file, but seems a bit much.

Looks like your theme doesn't use --bold in fish_color_redirection like the default one does?
That's probably better, so maybe we should change the default themes accordingly. Redirection should probably still be different from fish_color_param. We have some underline styles available now, which we can use on most terminals

@Ordoviz Ordoviz force-pushed the underline_valid_path_redirection branch from da15ee6 to 302a2bb Compare January 4, 2026 18:00
@Ordoviz
Copy link
Contributor Author

Ordoviz commented Jan 4, 2026

Looks like your theme doesn't use --bold in fish_color_redirection like the default one does?

Yes, my theme contains set fish_color_redirection normal, but I don't think the default theme needs to be changed. In the foot terminal emulator, the bold+underline combinations looks good:
image

I think this is ready to be merged once CHANGELOG.rst has been updated.

skip-checks: true
@krobelus
Copy link
Contributor

krobelus commented Jan 5, 2026

which font settings are you using?
I think my screenshot is with font=monospace:size=12, mostly foot defaults and

$ fc-match monospace
NimbusMonoPS-Regular.otf: "Nimbus Mono PS" "Regular"

@krobelus
Copy link
Contributor

krobelus commented Jan 5, 2026

nevermind, it was just due to 1.2x fractional (compositor-level) scaling on my end, so it's all good

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.

2 participants