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

lib/theme: shfmt, shellcheck, and some cleanup #2038

Merged
merged 11 commits into from
Mar 4, 2022

Conversation

gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Jan 12, 2022

Description

Quote a bunch of things, adopt shellcheck recommendations, try to simplify a few parts...

Motivation and Context

Simplicity, clarity, and #1696!

How Has This Been Tested?

This is part of my main branch since September and the full test suite passes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • If my change requires a change to the documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • I have added tests to cover my changes, and all the new and existing tests pass.

@gaelicWizard gaelicWizard marked this pull request as ready for review January 12, 2022 22:03
@gaelicWizard gaelicWizard marked this pull request as draft January 12, 2022 22:03
@gaelicWizard gaelicWizard marked this pull request as ready for review January 12, 2022 22:10
@gaelicWizard gaelicWizard force-pushed the theme/base branch 6 times, most recently from 150bdfd to 43ef5c9 Compare January 25, 2022 06:30
@gaelicWizard gaelicWizard force-pushed the theme/base branch 3 times, most recently from 56c2d83 to 8616ae4 Compare January 30, 2022 07:20
@gaelicWizard gaelicWizard force-pushed the theme/base branch 3 times, most recently from 43084a9 to e32f001 Compare February 13, 2022 23:20
@gaelicWizard gaelicWizard force-pushed the theme/base branch 2 times, most recently from afe9142 to a656f09 Compare February 23, 2022 23:18
Copy link
Member

@NoahGorny NoahGorny left a comment

Choose a reason for hiding this comment

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

LGTM

gaelicWizard and others added 10 commits March 3, 2022 22:53
These variables are referenced by themes already linted.
My apologies to future `git blame` hunters ♥
My apologies to future `git blame` hunters ♥
My apologies to future `git blame` hunters ♥

Use the "short" host name by default (`\h`), not the fully qualified domain name (`\H`)...

lib/theme: don't redefine battery_char()

Combine the two definitions for `battery_char()` so the second one doesn't just overwrite the first one. Do one or the other, not both.

Don't evaluate if `battery_percentage()` is available at load time, evaluate it at run time.

Don't run `date` for `$THEME_TIME_FORMAT`, use `\D{fmt}`.
Five years deprecation is plenty warning.
A lot of useless `echo`s in here.
Improve handling of parameters by adding defaults (often blank).

Alsö, eliminate newlines from `echo` in many places.
- Don't invoke the source control utility when all we want to know is if we're somewhere inside the repository; use `_bash-it-find-in-ancestor()`.
@NoahGorny NoahGorny merged commit 53e5965 into Bash-it:master Mar 4, 2022
@gaelicWizard gaelicWizard deleted the theme/base branch March 4, 2022 19:28
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