Irregular verbs #2285
Irregular verbs #2285
Conversation
elijah-potter
left a comment
elijah-potter
left a comment
There was a problem hiding this comment.
Overall, I love this refactor. I just have a few comments related to discoverability. Thanks!
| @@ -0,0 +1,120 @@ | |||
| use lazy_static::lazy_static; | |||
There was a problem hiding this comment.
This module is not directly related to linting. Could we instead move it next to the dictionary or perhaps simply higher up in the hierarchy?
There was a problem hiding this comment.
Most of the dictionary/annotation/metadata logic seems to be in /src/ with some in /spell/. Since the latter would definitely be wrong I moved irregular_verbs.rs to /src/ - and it's now joined by irregular_nouns.rs
| @@ -0,0 +1,127 @@ | |||
| [ | |||
| "// comments can appear in the line before an entry", | |||
There was a problem hiding this comment.
I adore the idea of having a dedicating file for this, similar to the dictionary. I'd like to centralize all data files, though. Would you mind moving this next to the dictionary.dict?
There was a problem hiding this comment.
Done. And there's also irregular_nouns.json now.
elijah-potter
left a comment
elijah-potter
left a comment
There was a problem hiding this comment.
This is so close. Love the addition of the noun stuff.
| (*NOUNS).clone() | ||
| } | ||
|
|
||
| pub fn get_plural_for_singular(&self, singular: &str) -> Option<&str> { |
There was a problem hiding this comment.
I expect that we'll want to eventually have versions of these plugins that accept and return char arrays. I think we're fine for now, but it's something to think about.
There was a problem hiding this comment.
Yes I've been thinking of various things that can be added but adding when they're needed seems pragmatic.
There was a problem hiding this comment.
Yes I've been thinking of various things that can be added but adding when they're needed seems pragmatic.
harper-core/src/irregular_verbs.rs
Outdated
| Ok(Self { verbs }) | ||
| } | ||
|
|
||
| pub fn get() -> Arc<Self> { |
There was a problem hiding this comment.
Can we name this function similarly to the curated dictionary's equivalent?
There was a problem hiding this comment.
I did start with the same curated() which I didn't think made sense for this. But then again they are actually curated!
harper-core/src/lib.rs
Outdated
| pub mod expr; | ||
| mod fat_token; | ||
| mod ignored_lints; | ||
| pub mod irregular_nouns; |
There was a problem hiding this comment.
Let's just export the structs directly. Having the extra layer of indirection is redundant.
There was a problem hiding this comment.
Let's just export the structs directly. Having the extra layer of indirection is redundant.
At first I wasn't sure what this meant, and neither was the LLM. But have a look - I think I got it how you want it.
elijah-potter
left a comment
elijah-potter
left a comment
There was a problem hiding this comment.
Looks good. Thanks!
2 participants
Issues
N/A
Description
dictionary.dictandannotations.jsonare loaded.SimplePastToPastParticiplelinter that uses it.WillNonLemma(feat: will ran → will run / ran work in progress #2268) andExtraneousDidPastlinters.How Has This Been Tested?
Unit tests are included for the one API so far needed.
Checklist