Clean up 10 theme files and expand linting coverage #2334
Conversation
* master: (767 commits) Tofu completion rewrite have pre-commit ignore the /vendor, they should be immutable. Fix: __powerline_last_status_prompt to handle unset argument safely remove superfluous function and pick better var names for readability Update base.theme.bash Update base.theme.bash Update base.theme.bash Update base.theme.bash Fix for 2323 issue Apply fixes Add `.git-blame-ignore-revs` Clean themes A-L Update powerline-multiline.base.bash Fix a couple bugs introduced by last commit Fix bad merge Update plugins/available/extract.plugin.bash important syntax correction from the owenr of ble.sh only the correct FZF integration loads, sepending on whether the blesh plugin is enabled as well. readonly HIST* variables is a bit extreme and clashes with ble.sh and other tools. implement #2275 more elegantly ...
• Fix color variable references using ${var?} pattern for fail-fast behavior
• Resolve SC2155 variable declaration/assignment separation issues
• Fix SC2181 exit code checking and SC2059 printf format issues
• Add themes/base.theme.bash, themes/mairan, themes/mbriggs, themes/metal, themes/minimal, themes/modern-t to clean_files.txt
• All themes now pass shellcheck, shfmt, and bash-it requirements
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <[email protected]>
• Fix color variable references using ${var?} pattern for robust error handling
• Resolve SC2181 exit code checking and SC2236 style issues in modern-time theme
• Remove problematic shebang lines from theme files per bash-it standards
• Add themes/modern-time, themes/morris, themes/n0qorg, themes/newin, themes/nwinkler to clean_files.txt
• All themes pass shellcheck, shfmt, and bash-it requirements
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <[email protected]>
|
Exciting times. I am a humanist, never wanted to own a slave, but renting a cyber-slave for $20 a month feels addictive... |
|
@akinomyoga what do you think? we have about 30 more themes to clean. I thought about doing 4 PRs with 10 in each, or dump all 40 into one PR. got a pref? |
There was a problem hiding this comment.
we have about 30 more themes to clean. I thought about doing 4 PRs with 10 in each, or dump all 40 into one PR. got a pref?
Either works for me.
In general, if PRs are to be separated, the same type of fixes should be grouped and applied to all themes at once, (instead of grouping fixes based on the themes that they belong to).
| local user_host | ||
| user_host="${green?}\h${reset_color?}" |
There was a problem hiding this comment.
Is this needed? When the right-hand side contains command substitutions $(...), Shellcheck will complain that the exit status of the command is masked. However, this line doesn't contain any command substitutions.
There was a problem hiding this comment.
but shellcheck did complain. you prefer I disable a few more checks?
There was a problem hiding this comment.
What kind of error does Shellcheck produce?
I tried it in my environments, but Shellcheck doesn't complain about this. Which version of Shellcheck do you use?
$ shellcheck --version
ShellCheck - shell script analysis tool
version: 0.10.0
license: GNU General Public License, version 3
website: https://www.shellcheck.netIn another host,
$ shellcheck --version
ShellCheck - shell script analysis tool
version: 0.11.0
license: GNU General Public License, version 3
website: https://www.shellcheck.net| @@ -33,14 +35,15 @@ modern_current_time_prompt() { | |||
|
|
|||
| prompt() { | |||
| SCM_PROMPT_FORMAT='[%s][%s]' | |||
There was a problem hiding this comment.
| SCM_PROMPT_FORMAT='[%s][%s]' | |
| local last_status=$? | |
| SCM_PROMPT_FORMAT='[%s][%s]' |
| local last_status=$? | ||
| if [ $last_status -ne 0 ]; then |
There was a problem hiding this comment.
| local last_status=$? | |
| if [ $last_status -ne 0 ]; then | |
| if ((last_status != 0)); then |
| . "$BASH_IT/themes/powerline/powerline.base.bash" | ||
|
|
||
| function __powerline_left_segment { | ||
| local OLD_IFS="${IFS}" | ||
| IFS="|" | ||
| local params=($1) | ||
| local -a params | ||
| IFS='|' read -ra params <<< "$1" |
There was a problem hiding this comment.
This fails to produce the expected result when $1 contains newlines.
Co-authored-by: Koichi Murase <[email protected]>
Summary
• Fix shellcheck issues in 10 theme files using proper
${var?}pattern for robust color variable handling• Resolve SC2155 variable declaration/assignment separation, SC2181 exit code checking, and other style issues
• Add themes to clean_files.txt to expand gradual linting system coverage
Files cleaned
First batch (5 themes):
Second batch (5 themes):
Test plan
🤖 Generated with Claude Code