Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

add new option "POWERLEVEL9K_BATTERY_HIDE_ABOVE_THRESHOLD" #830

Merged
merged 4 commits into from Jun 25, 2018

Conversation

iilonmasc
Copy link
Member

Hides the battery segment if percentage hits 100%

Hides the battery segment if percentage hits 100%
@iilonmasc
Copy link
Member Author

If you add POWERLEVEL9K_BATTERY_HIDE_FULL=true to your p9k config the battery segment will be hidden if the percentage is equal to 100%

Copy link
Member

@dritter dritter left a comment

Choose a reason for hiding this comment

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

Thanks @sambadevi for the PR. I left a word on how to get rid of the code duplication..

local offset=$(( ($bat_percent / $segment) + 1 ))
"$1_prompt_segment" "$0_${current_state}" "$2" "${POWERLEVEL9K_BATTERY_LEVEL_BACKGROUND[$offset]}" "${battery_states[$current_state]}" "${message}" "BATTERY_ICON"
if [[ -v "POWERLEVEL9K_BATTERY_HIDE_FULL" && "$POWERLEVEL9K_BATTERY_HIDE_FULL" == true ]]; then
if [[ "$bat_percent" != "100" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be easier to inverse that condition and exit early?
Something like this:

if [[ "${POWERLEVEL9K_BATTERY_HIDE_FULL}" == "true" && "${bat_percent}" == 100 ]]; then
  return
fi

That way we get rid of that indentations and the code duplication..

@dritter
Copy link
Member

dritter commented May 8, 2018

Btw. Something itches my head: I always try to reduce the amount of variables make variables more meaningful in P9K. So it might be worth it to rename that variable to POWERLEVEL9K_BATTERY_HIDE_ABOVE_THRESHOLD and make the value an integer. That way we would gain more flexibility without making the code more complex..

What do you think?

@iilonmasc
Copy link
Member Author

Thanks! I didn’t think about the code duplication. I will take a look at this later on and update the PR accordingly

Variable now holds an integer, if your battery status is greater or equal to this integer the
segment will be hidden
@iilonmasc iilonmasc changed the title add new option "POWERLEVEL9K_BATTERY_HIDE_FULL" add new option "POWERLEVEL9K_BATTERY_HIDE_ABOVE_THRESHOLD" May 9, 2018
Copy link
Member

@dritter dritter left a comment

Choose a reason for hiding this comment

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

Looks much better! Thx @sambadevi

# Draw the prompt_segment
"$1_prompt_segment" "$0_${current_state}" "$2" "${DEFAULT_COLOR}" "${battery_states[$current_state]}" "${message}" "BATTERY_ICON"
fi
if [[ -v "POWERLEVEL9K_BATTERY_HIDE_ABOVE_THRESHOLD" && "${bat_percent}" -ge $POWERLEVEL9K_BATTERY_HIDE_ABOVE_THRESHOLD ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment before that line? Something like:
"Check if POWERLEVEL9K_BATTERY_HIDE_ABOVE_THRESHOLD is set and is exceeded by bat_percent, early exit."

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I’ll add the comment

@dritter dritter mentioned this pull request Jun 7, 2018
@bhilburn bhilburn merged commit 7ab9cb1 into Powerlevel9k:next Jun 25, 2018
@iilonmasc iilonmasc deleted the feature/hide-battery-segment branch March 13, 2019 18:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants