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

Add option to display battery status as hearts in battery.sh #447

Merged
merged 4 commits into from
Oct 10, 2024

Conversation

nikunjmathur08
Copy link
Contributor

#438

This PR restores the "hearts" display for the cute battery mode, as mentioned in issue #438. It also introduces a new symbol option to display the battery status with a battery symbol.

@xx4h
Copy link
Collaborator

xx4h commented Oct 7, 2024

Thanks a lot for your work!

You're using BATTERY_FULL and BATTERY_EMPTY inside of __generate_hearts() which will just use the battery symbols and not the hearts. I suggest we bring back HEART_FULL and HEART_EMPTY, removed by this commit: cab0747

Besides that, I just have 3 small remarks:

  1. Add the missing ;; mentioned by the linter
  2. Change TMUX_POWERLINE_SEG_BATTERY_NUM_BATTERIES_DEFAULT to TMUX_POWERLINE_SEG_BATTERY_NUM_HEARTS_DEFAULT in
    TMUX_POWERLINE_SEG_BATTERY_NUM_BATTERIES_DEFAULT=5
    and
    export TMUX_POWERLINE_SEG_BATTERY_NUM_HEARTS="${TMUX_POWERLINE_SEG_BATTERY_NUM_BATTERIES_DEFAULT}"
    so it matches the other variable and the use case.
  3. In
    # How to display battery remaining. Can be {percentage, cute}.
    add the new hearts option to the doc line?

@nikunjmathur08
Copy link
Contributor Author

Made the following changes:

  • Reintroduced HEART_FULL and HEART_EMPTY variables for displaying hearts.
  • Renamed TMUX_POWERLINE_SEG_BATTERY_NUM_BATTERIES_DEFAULT to TMUX_POWERLINE_SEG_BATTERY_NUM_HEARTS_DEFAULT.
  • Added the 'hearts' option to the TMUX_POWERLINE_SEG_BATTERY_TYPE documentation.
  • Fixed the case statement by adding the missing ;; after the "hearts" case.

Fixes #438

segments/battery.sh Outdated Show resolved Hide resolved
@nikunjmathur08
Copy link
Contributor Author

I've also removed the extra 7th line.

segments/battery.sh Outdated Show resolved Hide resolved
Co-authored-by: Fabian Sylvester <[email protected]>
@xx4h
Copy link
Collaborator

xx4h commented Oct 10, 2024

LGTM, tested on linux.

Thanks again @nikunjmathur08!

@xx4h xx4h merged commit 1043cd9 into erikw:main Oct 10, 2024
1 check passed
@nikunjmathur08
Copy link
Contributor Author

@xx4h can you please add this under Hacktoberfest?

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