Theme/tokyonight - #2404
Conversation
|
It seems that |
|
yeah, that's not good, the ci does a setup of a specific go version, but try to install the latest shfmt version. I'll make a PR to fix that. |
Thanks so much. |
|
merge from master now, #2405 fixed the CI. |
|
sorry, I replied from the wrong user :) |
seefood
left a comment
seefood
left a comment
There was a problem hiding this comment.
Thanks for the theme! Found a couple of issues before this can merge:
Blocking — will fail CI
themes/tokyonight/tokyonight.theme.bash:13—. /etc/os-releasetriggers shellcheck SC1091 ("not following") with no disable comment. Every other file in the codebase that sources an external file (nvm.plugin.bash,rvm.plugin.bash,virtualenv.plugin.bash,powerline-naked.base.bash, etc.) adds# shellcheck disable=SC1091for exactly this reason. Runningshellcheckon this file locally confirms it exits non-zero, and there's no.shellcheckrc/severity override in this repo to mask it — the pre-commit shellcheck hook will fail on this PR as submitted.
Worth addressing
- Same line — sourcing
/etc/os-releasewithout localizing its variables leaksID,NAME,VERSION,PRETTY_NAME, etc. into global scope, and re-parses the file on every single prompt render (sincedistro_prompt_inforuns viaPROMPT_COMMAND). Since the distro doesn't change during a session, consider caching the parsed value or scoping the variables to avoid polluting the user's shell namespace.
Nit
docs/themes-list/tokyonight.rst:26has trailing whitespace before the. ~/.bashrccode block (not CI-blocking, just a stray artifact).
Everything else looks good — shfmt is clean, no naming collisions with other themes' helper functions, and the RST doc renders correctly.
|
Thanks a lot for the review. It is late here. I will address these issues tomorrow. |
- Address PR issues - Optimize code structure - Change the behavior of prompt character: color for exit code and shape for user
Move shellcheck to the right place
The caches for nodejs and rust may return out-of-date versions
|
I have addressed the issues mentioned above. Please review again. |
seefood
left a comment
seefood
left a comment
There was a problem hiding this comment.
not ideal but acceptable. LMK if you want me to merge or if you want to have another go at improving the metadata cache.
| if [[ -n $(command -v node) && -d "node_modules" ]]; then | ||
| echo " $(node -v) " | ||
| fi | ||
| } | ||
|
|
||
| function python_prompt_info() { | ||
| if [[ -n "$VIRTUAL_ENV" ]]; then | ||
| echo " ${VIRTUAL_ENV##*/} " | ||
| fi | ||
| } | ||
|
|
||
| function rust_prompt_info() { | ||
| if [[ -n $(command -v rustc) && -f "Cargo.toml" ]]; then | ||
| echo " $(rustc --version | awk '{print $2}') " |
There was a problem hiding this comment.
do note you are reading /etc/os-release on each and every prompt, and running rustc and node. it's a bit heavy. maybe consider caching it? just a thought.
|
@hiedanoajuu still working on it or should I merge? |
3 participants
Description
Add a new theme
tokyonightforbash-it.Motivation and Context
Based on bash-it's
easytheme, this themetokyonightreplicates the look of Tokyo Night Preset for Starship.How Has This Been Tested?
~/.bashrcScreenshots (if appropriate):
Types of changes
Checklist:
clean_files.txtand formatted it usinglint_clean_files.sh.