fix: pre-commit misc fixes #429
Conversation
The patch fixes command line when used as pre-commit plugin and change the hook. The --edit flag was asking for a argument which was not supplied by pre-commit. When using prepare-commit-msg hook, it was impossible to use an editor for the commit message.
WalkthroughThe update modifies the pre-commit hook configuration for commit message linting. Specifically, it changes the execution command for the commitlint tool to explicitly reference the commit message file and adjusts the hook's trigger stage from Changes
Poem
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:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.pre-commit-hooks.yaml (1)
9-9: Reconsider enabling verbose output
Settingverbose: truewill echo the full command on every commit. If it isn’t needed for debugging, disabling verbose mode (verbose: false) can keep the hook output cleaner.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.pre-commit-hooks.yaml(1 hunks)
🔇 Additional comments (1)
.pre-commit-hooks.yaml (1)
6-6: Confirm the correct hook stage for message linting
Switching fromprepare-commit-msgtocommit-msgis the right lifecycle event for validating the final commit message. This ensures linting runs after the message is composed.
| name: Assert Conventional Commit Messages | ||
| description: 'Asserts that Conventional Commits have been used for all commit messages according to the rules for this repo.' | ||
| entry: commitlint --edit | ||
| entry: commitlint --edit .git/COMMIT_EDITMSG |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Use a dynamic commit message argument instead of a hard-coded path
Hard‑coding .git/COMMIT_EDITMSG can break in environments with custom Git dirs or submodules. Let pre‑commit pass the correct commit-msg file via $1 and restore the default filename passing:
-entry: commitlint --edit .git/COMMIT_EDITMSG
+entry: commitlint --edit "$1"Also update or remove pass_filenames: false so that the hook receives the filename as its first argument.
Update commitlint hook to use the dynamic commit‐message file
Hard‑coding .git/COMMIT_EDITMSG can break in repos with custom Git dirs or submodules. In your .pre-commit-hooks.yaml:
• File: .pre-commit-hooks.yaml
• Line: 4
Replace:
-entry: commitlint --edit .git/COMMIT_EDITMSG
+entry: commitlint --edit "$1"And ensure you keep:
pass_filenames: falseso that pre‑commit passes only the commit‑message file ($1) and no other filenames.
There was a problem hiding this comment.
It failed when I gave the proposal a try.
There was a problem hiding this comment.
🧩 Analysis chain
@regit I'll need a bit more information to help you solve this issue. Could you share what error you encountered when trying the "$1" approach?
Let me also check how the command-line arguments are processed in this project to better understand the requirements.
🏁 Script executed:
#!/bin/bash
# Find how the --edit flag is implemented
rg --type rust "fn.*edit" -A 10
Length of output: 33
@regit Let me try different approaches to understand how the --edit flag works in this project.
🏁 Script executed:
#!/bin/bash
# Look for the CLI argument definitions that include the edit flag
rg --type rust "\-\-edit" -A 5 -B 5
Length of output: 754
@regit Let me continue investigating to better understand how the --edit flag works.
🏁 Script executed:
#!/bin/bash
# Find more information about the CLI arguments and specifically how the edit flag is defined
rg --type rust "edit.*=.*arg" -A 5 -B 5
Length of output: 41
@regit Let me try a different approach to find how the CLI arguments are defined.
🏁 Script executed:
#!/bin/bash
# Find how the CLI arguments are defined for the commitlint command
rg --type rust "struct.*Args" -A 20
Length of output: 1526
@regit Let me check how the edit argument is processed, which will help understand why it's failing.
🏁 Script executed:
#!/bin/bash
# Look at the processing of the edit argument
rg --type rust "if let Some\(edit\) = self\.edit\.as_deref\(\)" -A 10 -B 5
Length of output: 1076
@regit I understand better now. Looking at the code, I can see that the --edit flag needs a filename parameter. When you tried "$1", it likely failed because of quoting issues in the YAML file or how pre-commit passes arguments.
🏁 Script executed:
#!/bin/bash
# Let's look at how pre-commit handles hooks with commit-msg stage
# First check if there's any documentation in the repo
rg --type md "pre-commit" -A 10 -B 10
# Then look at any tests or examples with pre-commit hooks
ls -la
find . -name "*.toml" -o -name "*.yaml" -o -name "*.yml" | grep -i pre-commit
Length of output: 1169
@regit I think I understand the issue now. Looking at the code, the --edit flag requires a filename argument to read the commit message from.
When you tried using "$1", it likely failed because the quotes in the YAML file were interpreted literally - pre-commit would try to run commitlint --edit "$1" where it's looking for a file literally named $1 rather than substituting the value.
🏁 Script executed:
#!/bin/bash
# Let's check if .git/COMMIT_EDITMSG actually exists in a typical git repo
# and see if there are any other pre-commit examples with commit-msg stage
# Output the pre-commit-hooks.yaml file content
cat .pre-commit-hooks.yaml
# Check if there are any examples of pre-commit commit-msg hooks in other projects
rg --type yaml "\s+stages:\s+\[commit-msg\]" -A 5 -B 10
Length of output: 971
@regit I understand the issue now. When you tried using "$1" in the YAML file, it was treated as a literal string "$1" rather than a shell variable substitution.
For pre-commit hooks in the commit-msg stage, pre-commit passes the commit message file as the first argument. To use this, you need a wrapper script approach.
Here's what you could do:
- Create a small shell script in your repo:
#!/bin/sh
# pre-commit-commitlint.sh
commitlint --edit "$1"-
Make it executable:
chmod +x pre-commit-commitlint.sh -
Update your
.pre-commit-hooks.yaml:
- id: commitlint
name: Assert Conventional Commit Messages
description: 'Asserts that Conventional Commits have been used for all commit messages according to the rules for this repo.'
entry: pre-commit-commitlint.sh
language: script
stages: [commit-msg]
pass_filenames: false
require_serial: true
verbose: trueThis approach properly handles the commit message file passed by pre-commit to your script.
Alternatively, if you prefer not to add a script, keeping the hardcoded .git/COMMIT_EDITMSG path is a reasonable compromise that works in most cases, as you've implemented in your current PR.
The patch fixes command line when used as pre-commit plugin and change the hook.
The --edit flag was asking for a argument which was not supplied by pre-commit.
When using prepare-commit-msg hook, it was impossible to use an editor for the commit message.
Why
After "fixing" #428 by skipping compilation, I've found out that it was not working properly.
Summary by CodeRabbit