-
Notifications
You must be signed in to change notification settings - Fork 180
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
fix: Explicitly disable paging in pre-commit hook #3928
Conversation
The pre-commit hook that checks for leftover conflict markers introduced in acts-project#3708 calls git diff. While this works well in general, I am running into an edge case where git is failing to determine that it is not being run in a TTY, causing it to fire up a pager which, in turn, causes pre-commit to hang as it waits for the pager to close. This commit fixes this issue by adding the `--no-pager` flag to the git command.
WalkthroughChange made, yes. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
I am most likely just seeing this because I am an insufferable Linux hipster and I switched to the alacritty terminal emulator (implemented in Rust btw so it has no bugs). |
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
🧹 Outside diff range and nitpick comments (1)
.pre-commit-config.yaml (1)
94-95
: Document this wisdom for future generations, we should.Add a comment explaining why
--no-pager
flag necessary it is, prevent future confusion it will. Hmmmm.Apply this change, you should:
name: Leftover conflict markers language: system + # --no-pager flag prevents git from hanging when not in TTY entry: git --no-pager diff --staged --check
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.pre-commit-config.yaml
(1 hunks)
🔇 Additional comments (1)
.pre-commit-config.yaml (1)
95-95
: Approve this change, I do! Wise solution to prevent the hanging, it is.
Fixed the pager issue without affecting the conflict marker detection, this change has. Prevent the pre-commit hook from hanging indefinitely, it will. Hmmmm.
Search for other git commands that might benefit from this wisdom, we must:
✅ Verification successful
Complete, our investigation is! No other git commands requiring the --no-pager flag, there are.
Through the Force, seen I have that only one git command in the pre-commit hooks exists - the one we already fixed, it is. Safe and proper, this change remains. Mmmmm.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for other git commands in pre-commit hooks that might need --no-pager
rg "entry: git(?! --no-pager)" .pre-commit-config.yaml
Length of output: 295
Script:
#!/bin/bash
# Search for git commands in pre-commit hooks with proper regex
rg --pcre2 'entry:\s+git(?!\s+--no-pager)' .pre-commit-config.yaml
Length of output: 68
Script:
#!/bin/bash
# Alternative approach: just show all git commands in pre-commit hooks
rg 'entry:\s+git\s+' .pre-commit-config.yaml
Length of output: 96
Script:
#!/bin/bash
# Show the entire file content for manual inspection
cat .pre-commit-config.yaml
Length of output: 2440
|
This is a copy of acts-project/acts#3928; long story short, if git doesn't properly detect that it is not in a TTY, it will hang pre-commit. Therefore, this explicitly disables paging.
This is a copy of acts-project/acts#3928; long story short, if git doesn't properly detect that it is not in a TTY, it will hang pre-commit. Therefore, this explicitly disables paging.
The pre-commit hook that checks for leftover conflict markers introduced in #3708 calls git diff. While this works well in general, I am running into an edge case where git is failing to determine that it is not being run in a TTY, causing it to fire up a pager which, in turn, causes pre-commit to hang as it waits for the pager to close. This commit fixes this issue by adding the
--no-pager
flag to the git command.