-
Notifications
You must be signed in to change notification settings - Fork 346
feat(): Show repo name status bar #349
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
feat(): Show repo name status bar #349
Conversation
feat(): Display multiple timezones in the status bar
📝 WalkthroughWalkthroughA new tmux option Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Tmux
participant git.sh
User->>Tmux: Triggers Git status display
Tmux->>git.sh: Execute script
git.sh->>git.sh: Read @dracula-git-show-repo-name option
alt Option enabled and inside Git repo
git.sh->>git.sh: getRepoName()
git.sh->>git.sh: Prepend repo name to status output
else Option disabled or not in Git repo
git.sh->>git.sh: Skip repo name
end
git.sh-->>Tmux: Output status string (with or without repo name)
Tmux-->>User: Display status
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
scripts/git.sh (1)
160-169: Avoid redundantecho-into-variable pattern
output=$(echo "...")spawns an extra subshell for no gain. Direct assignment is simpler & faster:- output=$(echo "$repo_name ${changes} $branch") + output="$repo_name ${changes} $branch"Apply the same micro-optimisation to the other three similar lines in this block.
Pure style, but zero-cost to change.scripts/dual_time.sh (1)
20-25: Broaden zone-file test & quote expansionSome zones (e.g.
Etc/UTC) are directories, not plain files.-ehandles both and quoting silences SC2046.- if [ -f "/usr/share/zoneinfo/$tz" ]; then - time=$(env TZ="$tz" /bin/date +"${label} ${time_fmt}") + if [ -e "/usr/share/zoneinfo/$tz" ]; then + time="$(env TZ="$tz" /bin/date +"${label} ${time_fmt}")"scripts/dracula.sh (1)
44-47: Quote command substitutions when populating arraysUnquoted
$(get_tmux_option …)is benign for single-word defaults but will break if a user provides values containing spaces (e.g. plugin names with arguments).-IFS=' ' read -r -a plugins <<<$(get_tmux_option "@dracula-narrow-plugins" "compact-alt battery network weather") +IFS=' ' read -r -a plugins <<<"$(get_tmux_option "@dracula-narrow-plugins" "compact-alt battery network weather")"Apply the same quoting to the regular-width branch (line 46) for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/dracula.sh(9 hunks)scripts/dual_time.sh(1 hunks)scripts/git.sh(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
scripts/git.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
scripts/dracula.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
🪛 Shellcheck (0.10.0)
scripts/git.sh
[warning] 12-12: Quote this to prevent word splitting.
(SC2046)
[warning] 135-135: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 136-136: Quote this to prevent word splitting.
(SC2046)
[warning] 149-149: Quote this to prevent word splitting.
(SC2046)
[warning] 162-162: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 165-165: Quote this to prevent word splitting.
(SC2046)
[warning] 165-165: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 168-168: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 173-173: Quote this to prevent word splitting.
(SC2046)
[warning] 173-173: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 176-176: Expanding an array without an index only gives the first element.
(SC2128)
scripts/dracula.sh
[warning] 44-44: Quote this to prevent word splitting.
(SC2046)
[warning] 46-46: Quote this to prevent word splitting.
(SC2046)
[warning] 208-208: Quote this to prevent word splitting.
(SC2046)
[warning] 213-213: Quote this to prevent word splitting.
(SC2046)
[warning] 218-218: Quote this to prevent word splitting.
(SC2046)
[warning] 223-223: Quote this to prevent word splitting.
(SC2046)
[warning] 228-228: Quote this to prevent word splitting.
(SC2046)
[warning] 233-233: Quote this to prevent word splitting.
(SC2046)
[warning] 237-237: Quote this to prevent word splitting.
(SC2046)
[warning] 241-241: Quote this to prevent word splitting.
(SC2046)
[warning] 245-245: Quote this to prevent word splitting.
(SC2046)
[warning] 249-249: Quote this to prevent word splitting.
(SC2046)
[warning] 253-253: Quote this to prevent word splitting.
(SC2046)
[warning] 257-257: Quote this to prevent word splitting.
(SC2046)
[warning] 261-261: Quote this to prevent word splitting.
(SC2046)
[warning] 265-265: Quote this to prevent word splitting.
(SC2046)
[warning] 310-310: Quote this to prevent word splitting.
(SC2046)
[warning] 314-314: Quote this to prevent word splitting.
(SC2046)
[warning] 318-318: Quote this to prevent word splitting.
(SC2046)
[warning] 322-322: Quote this to prevent word splitting.
(SC2046)
[warning] 326-326: Quote this to prevent word splitting.
(SC2046)
[warning] 330-330: Quote this to prevent word splitting.
(SC2046)
[warning] 334-334: Quote this to prevent word splitting.
(SC2046)
[warning] 338-338: Quote this to prevent word splitting.
(SC2046)
[warning] 342-342: Quote this to prevent word splitting.
(SC2046)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
scripts/git.sh (3)
12-12: Stop reading@dracula-git-show-repo-nameinto an array – treat it as a scalar
The option is a single boolean string; usingread -r -aturns it into an array and triggers the same SC2128 warnings already raised earlier.-IFS=' ' read -r -a show_repo_name <<<$(get_tmux_option "@dracula-git-show-repo-name" "false") +show_repo_name="$(get_tmux_option "@dracula-git-show-repo-name" "false")"
169-172: Same spacing / array issue as aboveReplicate the fix applied to the diff-symbol branch to keep the output consistent and shellcheck-clean.
133-139: Quote$path, prepend the pipe, and drop the trailing bar
- Paths with spaces will explode without quoting.
- Spec says the bar precedes the repo name (
… | Repo), not trails (Repo |).- No need to expand an array without an index –
"$show_repo_name"is now a scalar (see previous comment).-getRepoName() -{ - if [ "$show_repo_name" == "true" ] && [ "$(checkForGitDir)" == "true" ]; then - repo="$(basename "$(git -C $path rev-parse --show-toplevel 2>/dev/null)")" - echo "$repo |" - fi -} +getRepoName() { + if [ "$show_repo_name" = "true" ] && [ "$(checkForGitDir)" = "true" ]; then + repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")" + echo "| $repo" + fi +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/git.sh(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/git.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
🪛 Shellcheck (0.10.0)
scripts/git.sh
[warning] 12-12: Quote this to prevent word splitting.
(SC2046)
[warning] 135-135: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 144-144: Quote this to prevent word splitting.
(SC2046)
[warning] 157-157: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 160-160: Quote this to prevent word splitting.
(SC2046)
[warning] 160-160: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 163-163: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 168-168: Quote this to prevent word splitting.
(SC2046)
[warning] 168-168: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 171-171: Expanding an array without an index only gives the first element.
(SC2128)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
scripts/git.sh (3)
133-139: Quote $path and move the pipe ingetRepoName
The two issues previously pointed out are still present:
git -C $path …breaks when the path contains spaces; quote the variable.- The spec shows
… | RepoName, but the current implementation returns"RepoName |".-getRepoName() -{ - if [ "$show_repo_name" == "true" ] && [ "$(checkForGitDir)" == "true" ]; then - repo="$(basename "$(git -C $path rev-parse --show-toplevel 2>/dev/null)")" - echo "$repo |" - fi -} +getRepoName() { + if [ "$show_repo_name" = "true" ] && [ "$(checkForGitDir)" = "true" ]; then + repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")" + echo "| $repo" + fi +}
155-158: Still building the string withecho, risking double spaces
output=$(echo …)was already flagged as fragile. Construct the string directly to avoid word-splitting quirks:- output=$(echo "$repo_name ${changes} $branch") + output="$repo_name${changes:+ ${changes}} $branch"Apply the same pattern to the other branches.
161-164:$diff_symbolused without an index – ShellCheck SC2128
diff_symbolis an array; use an explicit element or treat it as a scalar to silence the warning and avoid accidental truncation.- output=$(echo "$repo_name $diff_symbol $branch") + output="$repo_name ${diff_symbol[0]} $branch"
🧹 Nitpick comments (1)
scripts/git.sh (1)
169-172: Same array / extra-space issue forcurrent_symbolReplicate the fix for the no-changes path:
- output=$(echo "$repo_name $current_symbol $branch") + output="$repo_name ${current_symbol[0]} $branch"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/git.sh(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/git.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
🪛 Shellcheck (0.10.0)
scripts/git.sh
[warning] 144-144: Quote this to prevent word splitting.
(SC2046)
[warning] 160-160: Quote this to prevent word splitting.
(SC2046)
[warning] 160-160: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 163-163: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 168-168: Quote this to prevent word splitting.
(SC2046)
[warning] 168-168: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 171-171: Expanding an array without an index only gives the first element.
(SC2128)
🔇 Additional comments (1)
scripts/git.sh (1)
12-12: Scalar assignment looks goodSwitching to a straight assignment removes the SC2046/SC2128 warnings raised earlier.
Nothing further here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
scripts/git.sh (3)
133-139: Pipe is on the wrong side & trailing space will propagate
Spec (and the earlier review) expects| RepoName, notRepoName |.
Also, emitting a trailing space forces downstream callers to keep trimming or duplicating spaces.- repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")" - echo "$repo |" + repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")" + echo "| $repo"
168-171: Same array / quoting issue forcurrent_symbol-if [ $(checkEmptySymbol $current_symbol) == "true" ]; then +if [ "$(checkEmptySymbol "${current_symbol[0]}")" = "true" ]; thenIn the
elsebranch you already use${current_symbol[0]}—make the check consistent.
160-164: Unindexed$diff_symboltriggers SC2128 & mis-formats output-output=$(echo "$repo_name $diff_symbol $branch") +output="$repo_name ${diff_symbol[0]} $branch"This collapses the array, removes the useless subshell, and keeps spacing consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/git.sh(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/git.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
🪛 Shellcheck (0.10.0)
scripts/git.sh
[warning] 144-144: Quote this to prevent word splitting.
(SC2046)
[warning] 160-160: Quote this to prevent word splitting.
(SC2046)
[warning] 160-160: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 163-163: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 168-168: Quote this to prevent word splitting.
(SC2046)
[warning] 168-168: Expanding an array without an index only gives the first element.
(SC2128)
🔇 Additional comments (1)
scripts/git.sh (1)
12-12: Good: option now assigned as scalar
Switching to a simple assignment eliminates the earlier SC2046/SC2128 warnings – nice catch.
scripts/git.sh
Outdated
| if [ $(checkEmptySymbol $diff_symbol) == "true" ]; then | ||
| output=$(echo "${changes} $branch") | ||
| output="$repo_name${changes:+ ${changes}} $branch" | ||
| else | ||
| output=$(echo "$diff_symbol ${changes} $branch") | ||
| output="$repo_name ${diff_symbol[0]} ${changes:+$changes }$branch" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Array still expanded as scalar & double-space risk
$diff_symbol is an array; $diff_symbol[0] is already used in line 157, but the empty-symbol check on 154 still passes the entire array unquoted → SC2128/SC2046.
While here, redundant echo concatenation and inconsistent spacing creep in.
- if [ $(checkEmptySymbol $diff_symbol) == "true" ]; then
+ if [ "$(checkEmptySymbol "${diff_symbol[0]}")" = "true" ]; then(Apply the same quoting/indexing pattern everywhere checkEmptySymbol is called.)
Consider building output via plain assignment instead of echo to avoid accidental double spaces.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ $(checkEmptySymbol $diff_symbol) == "true" ]; then | |
| output=$(echo "${changes} $branch") | |
| output="$repo_name${changes:+ ${changes}} $branch" | |
| else | |
| output=$(echo "$diff_symbol ${changes} $branch") | |
| output="$repo_name ${diff_symbol[0]} ${changes:+$changes }$branch" | |
| fi | |
| if [ "$(checkEmptySymbol "${diff_symbol[0]}")" = "true" ]; then | |
| output="$repo_name${changes:+ ${changes}} $branch" | |
| else | |
| output="$repo_name ${diff_symbol[0]} ${changes:+$changes }$branch" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 154-154: Quote this to prevent word splitting.
(SC2046)
[warning] 154-154: Expanding an array without an index only gives the first element.
(SC2128)
🤖 Prompt for AI Agents
In scripts/git.sh around lines 154 to 158, the checkEmptySymbol function is
called with the entire diff_symbol array unquoted, causing array expansion
issues and potential double spaces in output. Fix this by passing only the first
element of diff_symbol properly quoted to checkEmptySymbol. Also, revise the
output variable assignment to use plain variable assignment with consistent
spacing instead of echo concatenation to prevent accidental double spaces. Apply
this quoting and indexing pattern consistently wherever checkEmptySymbol is
called with arrays.
- changed getRepoName to just return the repo name for clarity and reusability - moved pipe and spaces to separate into variable - removed space from behind repo_name - this is important to me because otherwise this widget would have an additional leading whitespace for everyone not using the repo_name option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/git.sh (1)
169-172: Unnecessary subshell & echo – just assign
Using$(echo …)adds a useless process and can mangle spacing. Direct assignment is simpler and safer:- output=$(echo "$repo_name$branch") + output="$repo_name$branch"Apply the same pattern throughout the block for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/git.sh(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/git.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
🪛 Shellcheck (0.10.0)
scripts/git.sh
[warning] 144-144: Quote this to prevent word splitting.
(SC2046)
[warning] 149-149: Quote this to prevent word splitting.
(SC2046)
[warning] 153-153: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 163-163: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 168-168: Quote this to prevent word splitting.
(SC2046)
[warning] 168-168: Expanding an array without an index only gives the first element.
(SC2128)
🔇 Additional comments (1)
scripts/git.sh (1)
12-12: Scalar read looks correct – no action needed.
The new option is retrieved as a plain scalar, which eliminates the earlier array/quoting warnings.
| if [ "$(checkEmptySymbol "${diff_symbol[0]}")" = "true" ]; then | ||
| output=$(echo "$repo_name$branch") | ||
| else | ||
| output=$(echo "$diff_symbol $branch") | ||
| output=$(echo "$repo_name$diff_symbol $branch") | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Still expanding an array as a scalar – SC2128 resurfaces
$diff_symbol is an array; using it unindexed ("$repo_name$diff_symbol $branch") expands only the first element and revives word-splitting warnings.
- output=$(echo "$repo_name$diff_symbol $branch")
+ output="$repo_name${diff_symbol[0]} $branch"Update any other occurrences to keep ShellCheck quiet and behaviour predictable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ "$(checkEmptySymbol "${diff_symbol[0]}")" = "true" ]; then | |
| output=$(echo "$repo_name$branch") | |
| else | |
| output=$(echo "$diff_symbol $branch") | |
| output=$(echo "$repo_name$diff_symbol $branch") | |
| fi | |
| if [ "$(checkEmptySymbol "${diff_symbol[0]}")" = "true" ]; then | |
| output=$(echo "$repo_name$branch") | |
| else | |
| output="$repo_name${diff_symbol[0]} $branch" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 163-163: Expanding an array without an index only gives the first element.
(SC2128)
🤖 Prompt for AI Agents
In scripts/git.sh around lines 160 to 164, the code uses the array diff_symbol
as a scalar without indexing, causing ShellCheck SC2128 warnings and
unpredictable word splitting. Fix this by explicitly referencing the intended
element of diff_symbol with an index (e.g., diff_symbol[0]) wherever it is used,
including in the else branch, to ensure consistent and predictable expansion and
silence ShellCheck warnings.
| getRepoName() | ||
| { | ||
| if [ "$show_repo_name" = "true" ] && [ "$(checkForGitDir)" = "true" ]; then | ||
| repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")" | ||
| echo "$repo" | ||
| fi | ||
| } | ||
|
|
||
| # return the final message for the status bar | ||
| getMessage() | ||
| { | ||
| if [ $(checkForGitDir) == "true" ]; then | ||
| branch="$(getBranch)" | ||
| repo_name="$(getRepoName) | " | ||
| output="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pipe placement & “empty-repo” glitch – output does not match spec
getRepoName returns the bare repo name, but a hard-coded " | " is appended later (repo_name="$(getRepoName) | ").
• When the option is disabled or you’re not in a Git repo, getRepoName returns nothing, yet you still inject " | " → stray bar.
• When the option is enabled, the final status becomes "Main | …" whereas the spec shows "| Main …" (bar precedes the name).
-getRepoName()
-{
- if [ "$show_repo_name" = "true" ] && [ "$(checkForGitDir)" = "true" ]; then
- repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")"
- echo "$repo"
- fi
-}
+getRepoName() {
+ if [ "$show_repo_name" = "true" ] && [ "$(checkForGitDir)" = "true" ]; then
+ repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")"
+ # Prepend the bar only when a repo name is present
+ echo "| $repo"
+ fi
+}
- repo_name="$(getRepoName) | "
+ repo_name="$(getRepoName)"This delivers the correct "| Main" format and avoids stray separators.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 144-144: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
In scripts/git.sh around lines 133 to 147, the getRepoName function returns the
repo name without a preceding pipe, but the calling code appends a hard-coded "
| " after it, causing stray separators when the repo name is empty and incorrect
formatting compared to the spec. To fix this, remove the " | " from the
assignment of repo_name and instead prepend the pipe inside getRepoName or in
the output formatting logic so that the pipe appears before the repo name only
when it exists, ensuring no stray separators and matching the spec format "|
Main".
and now this mistake is burned into the repo foreverrrr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
scripts/git.sh (2)
154-161: Array still expanded as scalar – SC2128 resurfaces.
diff_symbolis an array; line 163 passes the entire array ($diff_symbol) to the string, re-introducing ShellCheck warnings and unpredictable word-splitting.- output=$(echo "$repo_name$diff_symbol $branch") + output="$repo_name${diff_symbol[0]} $branch"Also, line 154 can avoid a subshell by dropping
$(checkEmptySymbol ...)’s command substitution:- if [ "$(checkEmptySymbol "${diff_symbol[0]}")" = "true" ]; then + if [ "$(checkEmptySymbol "${diff_symbol[0]}")" = "true" ]; then(Only minor, but keeps style uniform.)
Apply the same indexed/quoted pattern wherever
diff_symbolor other option arrays are used.
133-139: Separator placement & trailing-space bug – invert the pipe, drop the tailing space.
getRepoNamecurrently outputs"${repo} | "(repo first, pipe after the name, plus a trailing space).
Effects:
• Status becomesMain | …, whereas the spec example shows| Main …(pipe before name).
• Because the string already ends with a space, every consumer that prefixes its own space (${changes:+ ${changes}}etc.) yields double spaces.Recommended fix:
- repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")" - echo "$repo | " + repo="$(basename "$(git -C "$path" --no-optional-locks rev-parse --show-toplevel 2>/dev/null)")" + echo "| $repo"This puts the separator in the right place and guarantees exactly one space that downstream code can reuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/git.sh(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/git.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
🪛 Shellcheck (0.10.0)
scripts/git.sh
[warning] 144-144: Quote this to prevent word splitting.
(SC2046)
[warning] 149-149: Quote this to prevent word splitting.
(SC2046)
[warning] 153-153: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 163-163: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 168-168: Quote this to prevent word splitting.
(SC2046)
[warning] 168-168: Expanding an array without an index only gives the first element.
(SC2128)
🔇 Additional comments (1)
scripts/git.sh (1)
12-12: Good switch to scalar assignment.The option is now read as a simple string, eliminating the earlier array/quoting issues flagged by ShellCheck.
| if [ $(checkEmptySymbol $current_symbol) == "true" ]; then | ||
| output=$(echo "$branch") | ||
| output=$(echo "$repo_name$branch") | ||
| else | ||
| output=$(echo "$current_symbol $branch") | ||
| output="$repo_name${current_symbol[0]} $branch" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
current_symbol suffers from the same indexing / quoting issue.
checkEmptySymbol $current_symbol expands only the first element and drops quotes, triggering both SC2128 and SC2046.
- if [ $(checkEmptySymbol $current_symbol) == "true" ]; then
+ if [ "$(checkEmptySymbol "${current_symbol[0]}")" = "true" ]; thenMirror this fix everywhere current_symbol is referenced.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ $(checkEmptySymbol $current_symbol) == "true" ]; then | |
| output=$(echo "$branch") | |
| output=$(echo "$repo_name$branch") | |
| else | |
| output=$(echo "$current_symbol $branch") | |
| output="$repo_name${current_symbol[0]} $branch" | |
| fi | |
| if [ "$(checkEmptySymbol "${current_symbol[0]}")" = "true" ]; then | |
| output=$(echo "$repo_name$branch") | |
| else | |
| output="$repo_name${current_symbol[0]} $branch" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 168-168: Quote this to prevent word splitting.
(SC2046)
[warning] 168-168: Expanding an array without an index only gives the first element.
(SC2128)
🤖 Prompt for AI Agents
In scripts/git.sh around lines 168 to 172, the variable current_symbol is used
without proper quoting and indexing, causing shellcheck warnings SC2128 and
SC2046. To fix this, always reference current_symbol with explicit indexing and
proper quoting, for example use "${current_symbol[0]}" instead of
$current_symbol. Apply this quoting and indexing fix consistently wherever
current_symbol is referenced in this code block.
|
hi, |
Awesome thank you so much! and yeah I agree removing the extra white space is a good call. |
Show the repo name on the status bar like
tmux | MainCan be enabled and disabled by having
@dracula-git-show-repo-namein the config file. False by default.