Skip to content

Conversation

@Tijs-B
Copy link
Contributor

@Tijs-B Tijs-B commented Oct 15, 2025

Issues

There's no existing GitHub issue for this, but I noticed sometimes harper not seeing my typos when they appear in a git commit description after a comment.

Description

The previous gitcommit parser naively took the contents until the first comment. If there are inline comments, such as when squashing two commits in an interactive rebase, the text after those comments would not be checked.

Instead, we use tree-sitter-gitcommit parsing and mask the nodes that are subject, message lines or breaking change descriptions. As before, we re-parse these parts with the existing markdown parser.

A new crate harper-git-commit is added.

Demo

How Has This Been Tested?

Tested manually with helix and harper-ls on some git commits.

Checklist

  • I have performed a self-review of my own code
  • I have added tests to cover my changes

@Tijs-B Tijs-B force-pushed the push-xttpxwvuyukp branch 2 times, most recently from 38f6484 to dc2526c Compare October 15, 2025 13:00
@Tijs-B
Copy link
Contributor Author

Tijs-B commented Oct 15, 2025

I noticed cargo test is not being run in CI. I've changed the failing test in the last force push.

The previous gitcommit parser naively took the contents until the
first comment. If there are inline comments, such as when squashing two
commits in an interactive rebase, the text after those comments would
not be checked.

Instead, we use tree-sitter-gitcommit parsing and mask the nodes that
are subject, message lines or breaking change descriptions. As before,
we re-parse these parts with the existing markdown parser.

A new crate `harper-git-commit` is added.
@Tijs-B Tijs-B force-pushed the push-xttpxwvuyukp branch from dc2526c to 98013c8 Compare October 16, 2025 18:02
@elijah-potter
Copy link
Collaborator

I noticed cargo test is not being run in CI. I've changed the failing test in the last force push.

You're absolutely right. It must have gotten lost in #2037. I've returned it.

Copy link
Collaborator

@elijah-potter elijah-potter left a comment

Choose a reason for hiding this comment

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

Overall, this looks almost good to go.

[dependencies]
harper-core = { path = "../harper-core", version = "0.68.0" }
harper-tree-sitter = { path = "../harper-tree-sitter", version = "0.68.0" }
tree-sitter-gitcommit = { git = "https://github.com/gbprod/tree-sitter-gitcommit", rev = "a716678c0f00645fed1e6f1d0eb221481dbd6f6d" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, Harper can't include git dependencies. crates.io doesn't allow it. Would it be possible for you to upload the crate to crates.io and reference that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The crate is already on crates.io, but it does not have the newest commits in the repo unfortunately. I could ask for a release or use the released version there; however, the version there does not have separate tree-sitter nodes for comments in between actual message lines, meaning that I can't remove those comments from the mask.

I'll ask upstream for a new release and version bump on crates.io.

Sorry, I'm not really knowledgeable about the rust packaging ecosystem, and didn't know that git dependencies aren't allowed when publishing on crates.io.

@@ -0,0 +1,26 @@
This is the the subject

This is a first line without typos
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good tests!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants