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(themes/powerline-icon): support OMB_THEME_POWERLINE_ICON_CLOCK for the date string in segment user_info #600

Merged
merged 7 commits into from
Sep 10, 2024

Conversation

elg
Copy link
Contributor

@elg elg commented Sep 4, 2024

powerline-icon has a hardcoded time format while apparently THEME_CLOCK_FORMAT is the way to go. If not set, it seems to still be %H%M%S or something like that so there is nothing broken here.

Copy link
Contributor

@RobLoach RobLoach left a comment

Choose a reason for hiding this comment

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

This approach is used elsewhere, so should be good.

@akinomyoga
Copy link
Contributor

A subtle point is that THEME_CLOCK_FORMAT is already used to control the content of segment clock in the powerline themes. The clock segment can be enabled by specifying clock in POWERLINE_PROMPT. If we reuse the variable THEME_CLOCK_FORMAT to control also the format of the date embedded in the user_info segment, the content of clock segment is affected when the user wants to change the user_info, and the user_info is affected when the user wants to change the clock segment. If we would be going to provide the customizability, shouldn't we prepare a separate variable?

Also, the suggested change will affect the default behavior; %X %D and %H%M%S are different. The current default %X %D is not a random string but is used by the original author of this theme, so even if we provide customizability, we wouldn't want to change the default behavior unless there is a strong reason. Or we may ask the original author about the design change.

@TheWatcherMultiversal Do you have any opinion on the suggested change in the powerline-icon theme you contributed in #509?

@akinomyoga
Copy link
Contributor

rebased and resolved a conflict.

@akinomyoga akinomyoga changed the title Enable THEME_CLOCK_FORMAT support from .bashrc feat(themes/powerline-icon): support config OMB_THEME_POWERLINE_ICON_CLOCK for the date string in segment user_info Sep 10, 2024
@akinomyoga akinomyoga changed the title feat(themes/powerline-icon): support config OMB_THEME_POWERLINE_ICON_CLOCK for the date string in segment user_info feat(themes/powerline-icon): support OMB_THEME_POWERLINE_ICON_CLOCK for the date string in segment user_info Sep 10, 2024
@akinomyoga akinomyoga merged commit 8cc21b7 into ohmybash:master Sep 10, 2024
4 checks passed
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.

3 participants