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

fix: direnv individual message styling (#6153) #6389

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

0x57e11a
Copy link

Description

permits the direnv module's *_msg segments to use the formatter:

  • switches from map to map_meta
  • introduces a warning if the rc_path isn't UTF-8 (reasonably this shouldn't be an issue?)

Motivation and Context

as detailed in #6153, it's nice to have colors for quick glancing, and not having the ability to format segments is an inconsistency with other modules

closes #6153

Screenshots (if appropriate):

using the following config:

[direnv]
disabled = false

allowed_msg = "[allowed](bold green)"
not_allowed_msg = "[not allowed](bold yellow)"
denied_msg = "[denied](bold red)"

loaded_msg = "[loaded](bold green)"
unloaded_msg = "[unloaded](bold red)"

format = "$allowed $loaded"

before:
image
after:
image

How Has This Been Tested?

  • the msg_formatting test in the direnv module passes on linux
  • the above screenshots/config

testing should be trivial on other platforms, but these changes shouldn't be platform-dependent

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
    likely not necessary, as none of the other modules mention that their variables can also be formatted strings
  • I have updated the tests accordingly.

@0x57e11a
Copy link
Author

if really necessary, it shouldn't be too difficult to write a test to cover the non-utf8 case

@davidkna
Copy link
Member

davidkna commented Nov 16, 2024

I would prefer merging #6338 first to avoid this causing issues for conditionals.

Also, please just keep the rc_path behaviour as-is and refrain from moving the other variables to map_meta and keep the rest in map.

@0x57e11a
Copy link
Author

I would prefer merging #6338 first to avoid this causing issues for conditionals.

sounds good!

Also, please just keep the rc_path behaviour as-is and refrain from moving the other variables to map_meta and keep the rest in map.

moved!

@davidkna davidkna added the on hold Progress on this issue or PR is currently suspended. label Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold Progress on this issue or PR is currently suspended.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Direnv messages can't be styled individually
2 participants