Skip to content

Add verb past form only helpers#3379

Merged
elijah-potter merged 3 commits into
Automattic:masterfrom
impetus82:add-past-form-only-helpers
May 15, 2026
Merged

Add verb past form only helpers#3379
elijah-potter merged 3 commits into
Automattic:masterfrom
impetus82:add-past-form-only-helpers

Conversation

@impetus82
Copy link
Copy Markdown
Contributor

Summary

  • add DictWordMetadata::is_verb_simple_past_only
  • add DictWordMetadata::is_verb_past_participle_only
  • expose both helpers through TokenKind
  • cover irregular-only forms (ate, eaten) and ambiguous/regular forms (thought, walked)

Fixes #3372.

Testing

  • cargo fmt --package harper-core
  • cargo test -p harper-core past_only --lib
  • cargo test -p harper-core participle_only --lib
  • cargo test -p harper-core neither_past_form_only --lib
  • git diff --check

Copy link
Copy Markdown
Collaborator

@hippietrail hippietrail left a comment

Choose a reason for hiding this comment

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

It looks good to me. Your implementation was quite different from the mass noun one so I asked two AIs to compare them since I didn't trust my own Rust-foo.

Your test cases gave me an idea for a third you method you could trivially add that complements these.

}

#[test]
fn thought_is_neither_past_form_only() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey you know what would be even better, adding an is_verb_regular_past_form that does exactly this.

@impetus82
Copy link
Copy Markdown
Contributor Author

Thanks for the suggestion. I added is_verb_regular_past_form alongside the existing helpers, delegated it through TokenKind, and added coverage for walked plus irregular past forms (ate, eaten, thought).

Local checks run:

  • cargo fmt --package harper-core
  • cargo test -p harper-core regular_past --lib
  • cargo test -p harper-core past_only --lib
  • cargo test -p harper-core participle_only --lib
  • git diff --check

Comment thread harper-core/src/dict_word_metadata.rs Outdated
pub fn is_verb_regular_past_form(&self) -> bool {
self.verb.is_some_and(|v| {
v.verb_forms
.is_some_and(|vf| vf.contains(VerbFormFlags::PAST))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually I have a hunch that ::PAST might be unreliable. I think a more explicit way would be more trustworthy:

            v.verb_forms
                .is_some_and(|vf| vf.contains(VerbFormFlags::PRETERITE)
                               && vf.contains(VerbFormFlags::PAST_PARTICIPLE))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated it to use the explicit PRETERITE + PAST_PARTICIPLE check instead of relying on PAST. I also adjusted the regular-past tests around thought and kept the distinct-form checks for ate, eaten, and walked.

Local checks run:

  • cargo fmt --package harper-core
  • cargo test -p harper-core regular_past --lib
  • cargo test -p harper-core past_only --lib
  • cargo test -p harper-core participle_only --lib
  • git diff --check

Copy link
Copy Markdown
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.

This looks good to me too. Thanks!

@elijah-potter elijah-potter added this pull request to the merge queue May 15, 2026
Merged via the queue into Automattic:master with commit 51121cc May 15, 2026
13 checks passed
This was referenced May 15, 2026
This was referenced May 19, 2026
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.

Add an is_past_participle_only and maybe an is_simple_past_only method to DictWordMetadata and TokenKind

3 participants