Skip to content

fix: pre-commit misc fixes #429

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: pre-commit misc fixes #429

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .pre-commit-hooks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
- 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: commitlint --edit
entry: commitlint --edit .git/COMMIT_EDITMSG
Copy link

@coderabbitai coderabbitai bot Apr 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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: false

so that pre‑commit passes only the commit‑message file ($1) and no other filenames.

Copy link
Author

@regit regit Apr 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It failed when I gave the proposal a try.

Copy link

@coderabbitai coderabbitai bot Apr 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 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:

  1. Create a small shell script in your repo:
#!/bin/sh
# pre-commit-commitlint.sh
commitlint --edit "$1"

  1. Make it executable: chmod +x pre-commit-commitlint.sh

  2. 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: true

This 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.

language: rust
stages: [prepare-commit-msg]
stages: [commit-msg]
pass_filenames: false
require_serial: true
verbose: true
verbose: true