Skip to content
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.

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

feat(core): added a bunch more common rules #940

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

elijah-potter
Copy link
Collaborator

Description

I've gone ahead and added a bunch more phrase corrections.

Checklist

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

}

fn description(&self) -> &'static str {
"Corrects the archaic or mistaken `fro` to `for` when followed by a noun."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Corrects the archaic or mistaken `fro` to `for` when followed by a noun."
"Corrects the archaic or mistaken `fro` to `for`, or `from` when followed by a noun."

It could be a typo about fro used instead of from

I received a letter fro Sarah
I received a letter from Sarah

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, while fro is archaic, it's irrelevant since it's not an archaic form of for. It's an archaic form of from and is only used these days in to and fro.

I wonder if some logic for rare/archaic words could have a flag in the dictionary to allow them to get a warning and still undergo spellcheck. We'd want to find other instances similar to fro for that to be worthwhile. I know that back in the day spellcheck dictionaries purposefully left words out that were correct but rare and likely to cause confusion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would you suggest we change the description, or remove this rule as well as the dictionary entry for fro?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you suggest we change the description, or remove this rule as well as the dictionary entry for fro?

I would probably test if the spellcheck handles it. I've noticed it often doesn't make what I consider the 'obvious' suggestions for many mistakes. I haven't checked this one.

If the spellcheck suggests 'for' and 'from' we can probably do without the rule, but if it fails to suggest those then it's probably worth keeping the rule with a changed description.

At some point we probably want to add some code to 'score' suggestions in other ways than just pure edit distance to help get the best 3 into the suggestions.

Copy link
Collaborator Author

@elijah-potter elijah-potter Mar 25, 2025

Choose a reason for hiding this comment

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

That's been around for a while now. See harper-core/src/spell/mod.rs fn score_suggestion. I've tweaked it a bit, but I'm not sure how to achieve the desired behavior.

I think we should keep the rule with an updated description.

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

Successfully merging this pull request may close these issues.

3 participants