Theme/tokyonight - #2404
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
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
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.
|
@hiedanoajuu still working on it or should I merge? |
- Cache distro/nodejs/rust at startup - Tweak function and var name
|
I'm sorry that I have been busy with something else in the past few days. Please review it again. |
|
Is the metadata cache up to your expectations? Could you please merge it at your convenience? |
|
Yup, it's great, just GitHub threw 503 at me. I'll try again. |
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.