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

plugins: Add colored-man-pages plugin #619

Merged
merged 15 commits into from
Oct 2, 2024

Conversation

RobLoach
Copy link
Contributor

@RobLoach RobLoach commented Sep 30, 2024

Inspired by the colored-man-pages zsh plugin.

This is likely not the best design, as I'm grabbing the specific colors. If there's a better method, feel free to let me know.

Screenshot from 2024-09-30 16-53-12

Copy link
Contributor

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This plugin affects the global behavior of less instead of just affecting man.

I thought we might need to accept it if the upstream OMZ plugin of the same name were behaving in the same way. However, I checked the upstream plugin and realized that the upstream version correctly implements it using a shell function with the local environment variable so that it only affects man.

Could you adjust it so that it doesn't affect the global behavior of less?

@akinomyoga
Copy link
Contributor

Could you adjust it so that it doesn't affect the global behavior of less?

The upstream seems to be using env, but what I'm thinking is something like this:

function _omb_plugin_colored_man_pages_colored {
  local -x LESS_TERMCAP_mb=...
  ...

  command "$@"
}

_omb_util_binary_exists man &&
  function man { _omb_plugin_colored_man_pages_colored "$FUNCNAME" "$@"; }
_omb_util_binary_exists debman &&
  function debman { _omb_plugin_colored_man_pages_colored "$FUNCNAME" "$@"; }
...

Copy link
Contributor

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updates!

I have a question. This plugin and also the upstream one define the function colored, but is this something that this plugin intentionally exposes to the user? Is the function colored intended to be also directly used by users? It doesn't seem to be the case to me. If so, can we rename the function as _omb_plugin_colored_man_pages_colored? We try to avoid polluting the namespace of the commands that the users directly type and run. What do you think?

Ah, OK. Please disregard this. I noticed the usage of colored in README.md. Thank you!

plugins/colored-man-pages/colored-man-pages.plugin.sh Outdated Show resolved Hide resolved
akinomyoga
akinomyoga previously approved these changes Oct 2, 2024
@akinomyoga akinomyoga merged commit e712342 into ohmybash:master Oct 2, 2024
4 checks passed
@akinomyoga
Copy link
Contributor

@RobLoach Thanks!

@akinomyoga akinomyoga changed the title Add Colored Man Pages plugin plugins: Add colored-man-pages plugin Oct 2, 2024
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