[feature/git_helper-23] Added ability for repo specific GIT_HELPER_PREFIX#24
Merged
justinkumpe merged 1 commit intodevfrom Jan 7, 2026
Merged
[feature/git_helper-23] Added ability for repo specific GIT_HELPER_PREFIX#24justinkumpe merged 1 commit intodevfrom
justinkumpe merged 1 commit intodevfrom
Conversation
Reviewer's GuideImplements repo-specific commit message prefixes via a .GIT_HELPER_PREFIX file (with fallback to GIT_HELPER_PREFIX env var), adds a UI-driven command to configure prefixes either globally or per-repo, updates alias creation to touch both bashrc and bash_profile, and improves the TUI sizing and appearance for dialog/whiptail-based interactions. Sequence diagram for configuring commit prefix via utilities menusequenceDiagram
actor User
participant TUI as git_helper_TUI
participant Utils as menu_utils
participant Cmd as cmd_utils_set_prefix
participant Git as git
participant FS as filesystem
User->>TUI: Open Utilities menu
TUI->>Utils: menu_utils
Utils->>TUI: ui_menu("Utilities")
TUI-->>User: Show options
User-->>TUI: Select set_prefix
TUI->>Utils: choice set_prefix
Utils->>Cmd: cmd_utils_set_prefix
Cmd->>TUI: ui_menu("Set Prefix scope")
TUI-->>User: Choose local or repo
alt Local scope
User-->>TUI: Select local
TUI->>Cmd: scope local
Cmd->>FS: touch ~/.bashrc, ~/.bash_profile
Cmd->>TUI: ui_input("Enter GIT_HELPER_PREFIX")
TUI-->>User: Prompt for prefix
User-->>TUI: Enter prefix
TUI->>Cmd: prefix value
Cmd->>FS: Remove existing GIT_HELPER_PREFIX from profiles
Cmd->>FS: Append export GIT_HELPER_PREFIX
Cmd->>Cmd: export GIT_HELPER_PREFIX
Cmd->>TUI: ui_message("Prefix set, saved to profiles")
else Repo scope
User-->>TUI: Select repo
TUI->>Cmd: scope repo
Cmd->>Git: git rev-parse --is-inside-work-tree
Git-->>Cmd: is repo?
alt Not in repo
Cmd->>TUI: ui_message("Not in a git repository")
else In repo
Cmd->>Git: git rev-parse --show-toplevel
Git-->>Cmd: repo_root
Cmd->>FS: Read repo_root/.GIT_HELPER_PREFIX (if exists)
Cmd->>TUI: ui_input("Enter GIT_HELPER_PREFIX", current_prefix)
TUI-->>User: Prompt for repo prefix
User-->>TUI: Enter prefix
TUI->>Cmd: prefix value
Cmd->>FS: Write prefix to repo_root/.GIT_HELPER_PREFIX
Cmd->>Git: git check-ignore .GIT_HELPER_PREFIX
Git-->>Cmd: ignored or not
Cmd->>TUI: ui_message("Repo-specific prefix saved")
end
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
cmd_utils_add_alias, the previous behavior prompted before replacing an existing alias, but the new implementation silently removes any existing alias definitions in both~/.bashrcand~/.bash_profile; consider re-introducing a confirmation step or at least scoping replacement to the file where the alias was originally found to avoid surprising users. - The quote/whitespace stripping logic for
current_prefixin the repo-specific branch ofcmd_utils_set_prefixlooks fragile (${current_prefix#"${current_prefix%%[![:space:]]*}"}etc.) and hard to reason about; consider replacing it with a simpler, more explicit approach (e.g., using a singlesedor well-scoped parameter expansion) that doesn’t reuse the variable inside its own pattern. - Sourcing both
~/.bashrcand~/.bash_profileunconditionally incmd_utils_add_aliasandcmd_utils_set_prefixmay introduce side effects or duplicated initialization for some setups; you might want to check which file is actually used for interactive shells and only source that, or at least gate sourcing behind a confirmation prompt.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `cmd_utils_add_alias`, the previous behavior prompted before replacing an existing alias, but the new implementation silently removes any existing alias definitions in both `~/.bashrc` and `~/.bash_profile`; consider re-introducing a confirmation step or at least scoping replacement to the file where the alias was originally found to avoid surprising users.
- The quote/whitespace stripping logic for `current_prefix` in the repo-specific branch of `cmd_utils_set_prefix` looks fragile (`${current_prefix#"${current_prefix%%[![:space:]]*}"}` etc.) and hard to reason about; consider replacing it with a simpler, more explicit approach (e.g., using a single `sed` or well-scoped parameter expansion) that doesn’t reuse the variable inside its own pattern.
- Sourcing both `~/.bashrc` and `~/.bash_profile` unconditionally in `cmd_utils_add_alias` and `cmd_utils_set_prefix` may introduce side effects or duplicated initialization for some setups; you might want to check which file is actually used for interactive shells and only source that, or at least gate sourcing behind a confirmation prompt.
## Individual Comments
### Comment 1
<location> `git_helpers/git-helper.sh:750-751` </location>
<code_context>
+ local repo_root
+ repo_root=$(git rev-parse --show-toplevel 2>/dev/null || true)
+ if [[ -n "$repo_root" && -f "$repo_root/.GIT_HELPER_PREFIX" ]]; then
+ # Read first line and remove only trailing newline/carriage return (preserve spaces)
+ prefix=$(head -n 1 "$repo_root/.GIT_HELPER_PREFIX" 2>/dev/null | sed 's/[\r\n]*$//' || true)
+ fi
+ fi
</code_context>
<issue_to_address>
**issue (bug_risk):** The sed pattern for stripping CRLF may not reliably remove Windows-style carriage returns.
`sed 's/[\r\n]*$//'` depends on `\r` being treated as a carriage-return escape in basic regex, which isn’t portable across all `sed` implementations (some treat it as literal `r`). To make CRLF handling robust, prefer something like `sed 's/\r\?$//'` or `tr -d '\r' | head -n1`, and keep this consistent with `cmd_utils_set_prefix` so both paths parse the file the same way.
</issue_to_address>
### Comment 2
<location> `git_helpers/git-helper.sh:1798-1801` </location>
<code_context>
+ return
+ fi
+
+ local prefix_file="$repo_root/.GIT_HELPER_PREFIX"
+ local current_prefix=""
+ if [[ -f "$prefix_file" ]]; then
+ current_prefix=$(cat "$prefix_file" 2>/dev/null | head -n 1 || true)
+ current_prefix="${current_prefix#\"${current_prefix%%[![:space:]]*}\"}"
+ current_prefix="${current_prefix%\"${current_prefix##*[![:space:]]}\"}"
</code_context>
<issue_to_address>
**issue (bug_risk):** The logic for stripping quotes/whitespace from the existing prefix is complex and likely not doing what is intended.
These two parameter expansion lines are extremely hard to reason about and may not reliably “strip optional surrounding quotes/whitespace” as intended; they can also corrupt valid prefixes that legitimately contain quotes or spaces. Since you already read just the first line, consider reusing the simpler CRLF‑stripping approach from `expand_commit_prefix` (e.g., `head -n1 | sed 's/…$//'`) and avoid quote munging altogether, or replace this with a much simpler, clearly correct pattern if outer quotes truly need to be removed.
</issue_to_address>
### Comment 3
<location> `git_helpers/git-helper.sh:1733-1736` </location>
<code_context>
- # Source the profile to apply changes immediately
- source "$profile_file" 2>/dev/null || true
+ # Source both profiles to apply changes immediately
+ for profile_file in "${profile_files[@]}"; do
+ source "$profile_file" 2>/dev/null || true
+ done
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Sourcing both ~/.bashrc and ~/.bash_profile from a non-interactive script can introduce side effects.
This may trigger interactive-only setup (e.g., prompts, keybindings, long‑running commands) in a non-interactive context, leading to slowness or unexpected behavior. Consider either not sourcing and asking the user to open a new shell, sourcing only when an interactive shell is detected (e.g., `[[ $- == *i* ]]` or `[[ -n "$PS1" ]]`), or adding an explicit opt‑in flag to source profiles immediately.
```suggestion
# Source profiles only in interactive shells to avoid side effects in non-interactive contexts.
if [[ $- == *i* ]] || [[ -n "$PS1" ]]; then
for profile_file in "${profile_files[@]}"; do
# shellcheck source=/dev/null
source "$profile_file" 2>/dev/null || true
done
else
msg+=$'\nOpen a new shell or run the following to apply changes immediately:'
for profile_file in "${profile_files[@]}"; do
msg+=$'\n source '"$profile_file"
done
fi
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+750
to
+751
| # Read first line and remove only trailing newline/carriage return (preserve spaces) | ||
| prefix=$(head -n 1 "$repo_root/.GIT_HELPER_PREFIX" 2>/dev/null | sed 's/[\r\n]*$//' || true) |
There was a problem hiding this comment.
issue (bug_risk): The sed pattern for stripping CRLF may not reliably remove Windows-style carriage returns.
sed 's/[\r\n]*$//' depends on \r being treated as a carriage-return escape in basic regex, which isn’t portable across all sed implementations (some treat it as literal r). To make CRLF handling robust, prefer something like sed 's/\r\?$//' or tr -d '\r' | head -n1, and keep this consistent with cmd_utils_set_prefix so both paths parse the file the same way.
Comment on lines
+1798
to
+1801
| local prefix_file="$repo_root/.GIT_HELPER_PREFIX" | ||
| local current_prefix="" | ||
| if [[ -f "$prefix_file" ]]; then | ||
| current_prefix=$(cat "$prefix_file" 2>/dev/null | head -n 1 || true) |
There was a problem hiding this comment.
issue (bug_risk): The logic for stripping quotes/whitespace from the existing prefix is complex and likely not doing what is intended.
These two parameter expansion lines are extremely hard to reason about and may not reliably “strip optional surrounding quotes/whitespace” as intended; they can also corrupt valid prefixes that legitimately contain quotes or spaces. Since you already read just the first line, consider reusing the simpler CRLF‑stripping approach from expand_commit_prefix (e.g., head -n1 | sed 's/…$//') and avoid quote munging altogether, or replace this with a much simpler, clearly correct pattern if outer quotes truly need to be removed.
Comment on lines
+1733
to
+1736
| # Source both profiles to apply changes immediately | ||
| for profile_file in "${profile_files[@]}"; do | ||
| source "$profile_file" 2>/dev/null || true | ||
| done |
There was a problem hiding this comment.
suggestion (bug_risk): Sourcing both ~/.bashrc and ~/.bash_profile from a non-interactive script can introduce side effects.
This may trigger interactive-only setup (e.g., prompts, keybindings, long‑running commands) in a non-interactive context, leading to slowness or unexpected behavior. Consider either not sourcing and asking the user to open a new shell, sourcing only when an interactive shell is detected (e.g., [[ $- == *i* ]] or [[ -n "$PS1" ]]), or adding an explicit opt‑in flag to source profiles immediately.
Suggested change
| # Source both profiles to apply changes immediately | |
| for profile_file in "${profile_files[@]}"; do | |
| source "$profile_file" 2>/dev/null || true | |
| done | |
| # Source profiles only in interactive shells to avoid side effects in non-interactive contexts. | |
| if [[ $- == *i* ]] || [[ -n "$PS1" ]]; then | |
| for profile_file in "${profile_files[@]}"; do | |
| # shellcheck source=/dev/null | |
| source "$profile_file" 2>/dev/null || true | |
| done | |
| else | |
| msg+=$'\nOpen a new shell or run the following to apply changes immediately:' | |
| for profile_file in "${profile_files[@]}"; do | |
| msg+=$'\n source '"$profile_file" | |
| done | |
| fi |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Support repo-specific commit message prefixes and improve git-helper usability
New Features:
Enhancements:
Documentation: