Skip to content

Conversation

@hippietrail
Copy link
Collaborator

Issues

Resolves #2343

Description

In fact this finds 3 pairs of clashing linters already!

  • Intact - one in ClosedCompounds, the other in phrase_corrections.
  • Nobody - one in ClosedCompounds, the other standalone.
  • OfCourse - one standalone, the other in phrase_corrections.

I have intentionally not tried to resolve any of these as part of this PR.

I included another warning in this PR related to linters:

  • In harper-cli lint if after processing any --only and/or --ignore commandline arguments there are no lints configured, a warning will be issued. This is most commonly the result of making a typo when testing a new linter you're working on.

How Has This Been Tested?

Manually.

Checklist

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

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.

I like the idea, but I don't love having these methods do IO of any kind. Would it be possible to refactor this into a test case on the curated LintGroup?

@hippietrail
Copy link
Collaborator Author

I like the idea, but I don't love having these methods do IO of any kind. Would it be possible to refactor this into a test case on the curated LintGroup?

Putting this straight in my inbox for today!

@hippietrail
Copy link
Collaborator Author

I like the idea, but I don't love having these methods do IO of any kind. Would it be possible to refactor this into a test case on the curated LintGroup?

I made it a test by adding an Option<Vec> to LintGroup for linter names that clash. This way I was able to keep references to LintGroup as they were.

I took the opportunity to refactor the curated linter constructor too by adding new macros to add linters that use either a dialect or dictionary. But not for SpellCheck which, for now, is the only one that takes both.

I did not remove any of the three clashing Linters. Up to you if you want to roll that into this PR when you test it, or do it as a separate PR after this one.

Until then this PR will show up as failing of course!

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.

There are no errors or warnings if two Linters share the same name.

2 participants