WIP: wrap love - #250
Open
knocte wants to merge 32 commits into
Open
Conversation
Contributor
Author
|
@webwarrior-ws can you review? check each commit one by one, and I'm specially interested in c0f11d6 because I wonder what happens with Windows |
knocte
force-pushed
the
wip/wrapLove
branch
4 times, most recently
from
May 14, 2026 05:05
c69b4ef to
951485a
Compare
Contributor
Author
knocte
force-pushed
the
wip/wrapLove
branch
2 times, most recently
from
May 14, 2026 09:07
d90ac82 to
33c9959
Compare
Collaborator
|
I ran build on Windows, and tried to make a commit, but keep getting the following error: Even though I ran |
Contributor
Author
Using the same terminal that you were using when testing the above, can you run the script scripts/pre-commit.sh manually? |
Collaborator
I get a bash terminal window for a moment with the same error. |
Contributor
Author
So that's the first issue, we need a way to make that script crossplatform. |
webwarrior-ws
force-pushed
the
wip/wrapLove
branch
from
May 14, 2026 12:00
2fb6aad to
88bdf52
Compare
knocte
force-pushed
the
wip/wrapLove
branch
9 times, most recently
from
May 15, 2026 09:18
ca002b5 to
2d91597
Compare
MSBuild runs Exec commands from the project directory (e.g. src/FileConventions/), not the repo root, so 'dnx husky install' couldn't find .git. Adding WorkingDirectory=$(MSBuildThisFileDirectory) fixes this.
Instead of running 'dnx husky install' (which creates .husky/_/ and task-runner.json that we don't use), just set git config core.hooksPath =.husky directly. This avoids all Husky.NET infrastructure while still auto-configuring hooks for dotnet devs on local builds.
So if our hook scripts work directly on Windows via Git for Windows' built-in bash/sh (just as well as with Husky.NET's or npm husky's wrappers), then why use npm husky at all? What npm husky v9 actually provides: - HUSKY=0 env var to skip hooks. - PATH injection (node_modules/.bin) - ~/.huskyrc support. - Partial commit support (lint-staged integration). - 'husky add' / 'husky set' convenience commands. - Convention (it's the standard npm way). For our specific hooks, none of these features are needed: - pre-commit hook runs 'bash ./scripts/pre-commit.sh' which itself calls 'npm run format' and 'npx prettier', so it already resolves correctly. - post-commit runs dotnet fsi scripts/wrapLatestCommitMsg.fsx so it doesn't need PATH injection. - We don't use lint-staged. The real problem with keeping npm husky: Both npm husky (via 'prepare: husky') and our MSBuild target (via 'dotnet build') were fighting over core.hooksPath: - npm husky sets core.hooksPath=.husky/_ (with its _/ trampolines). Our MSBuild target was setting core.hooksPath=.husky (direct hooks). Whichever runs last wins. Both ultimately execute our .husky/pre-commit and .husky/post-commit, so it works, but it's confusing and fragile. So this commit fixes that: - npm devs: 'npm install' now runs 'git config core.hooksPath .husky' directly in the prepare script, instead of using the husky package. - dotnet devs: 'dotnet build' already runs the MSBuild target that does the same 'git config core.hooksPath .husky' (as per previous commit). Both ecosystems now use the exact same raw approach. No _/ directory, no task-runner.json, no npm husky dependency, no dnx dependency for hook installation. Just direct bash scripts in .husky/ that Git executes natively, which works on macOS, Linux, and Windows (via Git for Windows' built-in shell). NB: the way to guard against $CI being present (like we do in .NET) is to use a .ts script that checks it, otherwise the prepare step of package.json could not easily be crossplatform.
A $FOO text in the commit message triggered a shell expansion bug in wrapLatestCommitMsg.fsx. When the F# script ran git commit --amend --message "...$FOO...", the shell expanded $FOO (which was empty), effectively stripping the "$FOO" string from the amended message and corrupting the command. So, this fix: changed from git commit --amend --message "..." (value passed through shell, vulnerable to expansion) to writing the message to a temp file and using `git commit --amend --file <tempfile>`. This avoids all shell escaping issues entirely.
It seems a post-commit hook for amending commit messages cannot be used for cherry-pick operations, otherwise it would fail with: ``` fatal: You are in the middle of a cherry-pick -- cannot amend. ```
For bullet list.
To keep `*` bullet list entries on separate lines and not lump them together.
So that **BOLD** is not confused with bullet points.
That demonstrates that fix [1] should have used isColonBreak or something similar because it has failing CI. [1] 633ee15
For bullet list with hyphens.
Continuing the pairing with the commitlint's plugin tests.
The one about numeric bullets after colon.
One that I found while actually using the new commit hook with real work.
Testing in real world revealed an issue with the current algo which was not tracking if a text is inside the bullet list or not.
New .husky/commit-msg hook: - Replaces the old .husky/post-commit hook. - Receives the commit message file path ($1) and passes it to the F# script. - Because it's a commit-msg hook, if the script exits with a non-zero code, Git aborts the commit (unlike post-commit, which runs too late). Updated scripts/wrapLatestCommitMsg.fsx: - Reads the commit message from the file path passed as an argument (instead of git log -1 --format=%B). - Strips Git comment lines (# ...) before processing, then preserves them when writing back. - Validates the title length against the same limit as your commitlint policy (headerMaxLineLength = 50). If the title is too long, it prints an error to stderr and exits with code 1, blocking the commit. - Still wraps body paragraphs to 64 chars using the existing FileConventions.SafeWrapText logic. - Writes the result directly back to the commit message file, so no git commit --amend loop is needed. Why commit-msg and not pre-commit?: - The pre-commit hook runs before the commit message is even written. - It can't validate the message. - The commit-msg hook runs after the message is written but before the commit is finalized. - It can both modify the file and reject the commit.
With the advent of AI, it's very common to find em-dashes in their texts that we might copy+paste in the commit messages; however our wrapping logic might have conflicts with them if they are used as bullet points, and anyway they are very hard to differentiate them from normal dashes (at least in the font usually used to represent commit messages), which makes the texts a bit confusing to read; so let's just disallow them altogether.
TODO: we should probably stop exiting on the commit-msg-hook and just call commitlint there.
All inside the same test because they are very related. Now, I thought these tests would fail at first (so, TDD style), but they pass!, so I guess the AI made sure of this in the last commits.
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
2 participants
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.
No description provided.