Skip to content

example for adding a new rule#494

Closed
hippietrail wants to merge 5 commits intoAutomattic:masterfrom
hippietrail:example_new_rule
Closed

example for adding a new rule#494
hippietrail wants to merge 5 commits intoAutomattic:masterfrom
hippietrail:example_new_rule

Conversation

@hippietrail
Copy link
Copy Markdown
Collaborator

as per elijah-potter's request

See there for details on how I believe I've followed the instructions but my new rules is ignored and trying it only results in this:

hippietrail@macbookair harper % just lint ~/harper-test/harper-test.md
cargo run --bin harper-cli -- lint /Users/hippietrail/harper-test/harper-test.md
warning: /Users/hippietrail/harper/harper-cli/Cargo.toml: unused manifest key: package.private
warning: /Users/hippietrail/harper/harper-wasm/Cargo.toml: unused manifest key: package.private
   Compiling harper-core v0.16.0 (/Users/hippietrail/harper/harper-core)
   Compiling harper-tree-sitter v0.16.0 (/Users/hippietrail/harper/harper-tree-sitter)
   Compiling harper-typst v0.16.0 (/Users/hippietrail/harper/harper-typst)
   Compiling harper-html v0.16.0 (/Users/hippietrail/harper/harper-html)
   Compiling harper-comments v0.16.0 (/Users/hippietrail/harper/harper-comments)
   Compiling harper-literate-haskell v0.16.0 (/Users/hippietrail/harper/harper-literate-haskell)
   Compiling harper-cli v0.1.0 (/Users/hippietrail/harper/harper-cli)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 4.50s
     Running `target/debug/harper-cli lint /Users/hippietrail/harper-test/harper-test.md`
Advice: 
   ╭─[harper-test.md:1:1]
   │
 1 │ To reiterate, that that is cool is not uncool.
   │               ────┬────  
   │                   ╰────── “that that” sometimes means “that which”, which is clearer.
   │ 
 3 │ To reiterate, foo foo is cool is not uncool.
   │               ───┬───  
   │                  ╰───── Did you mean to repeat this word?
───╯

The added rule should look like the that-which rule but silly.

@elijah-potter
Copy link
Copy Markdown
Collaborator

@hippietrail, I've compiled your branch and it looks like it's working just fine.

Until now, if more than one rule finds an error harper-cli will only show the first one. If you open it up in Visual Studio Code or (in my case) Neovim, it shows both lints just fine.

That's actually a bug, and I've pushed a commit that should fix it 🤞. Now if you test using harper-cli, it'll show the FooBar.

@hippietrail
Copy link
Copy Markdown
Collaborator Author

hippietrail commented Jan 24, 2025

@hippietrail, I've compiled your branch and it looks like it's working just fine.

Until now, if more than one rule finds an error harper-cli will only show the first one. If you open it up in Visual Studio Code or (in my case) Neovim, it shows both lints just fine.

That's actually a bug, and I've pushed a commit that should fix it 🤞. Now if you test using harper-cli, it'll show the FooBar.

   ╭─[harper-test.md:1:1]
   │
 1 │ To reiterate, that that is cool is not uncool.
   │               ────┬────  
   │                   ╰────── “that that” sometimes means “that which”, which is clearer.
   │ 
 3 │ To reiterate, foo foo is cool is not uncool.
   │               ───┬───  
   │                  ╰───── “foo foo” sometimes means “foo bar”, bar is clearer.
───╯

OK nice. What about something like a --verbose switch that will show all matching rules and not just the first? As the number of rules grow there's going to a geometric increase in hidden ambiguities resulting in one rule chosen over others leading to such surprises, as anyone who has experimented with recursive descent parsing or parser combinators can attest.

Wait a sec... didn't you just say it should show both now?

I'll see if I can manage to test via LSP... Nope, sorry. Is that also in the new review docs? I can only see how to test via harper-cli and something about Docker that has a link that says it's for a demo but just takes me to part of a github repo??

@elijah-potter
Copy link
Copy Markdown
Collaborator

OK nice. What about something like a --verbose switch that will show all matching rules and not just the first?

I've set that up. You can either use just lint-verbose <file> or cargo run --bin harper-cli -- <file> --verbose

Are you happy enough with your FooBar rule that you want to elevate it to a real PR?

@hippietrail
Copy link
Copy Markdown
Collaborator Author

OK nice. What about something like a --verbose switch that will show all matching rules and not just the first?

I've set that up. You can either use just lint-verbose <file> or cargo run --bin harper-cli -- <file> --verbose

Are you happy enough with your FooBar rule that you want to elevate it to a real PR?

Not yet because though it's working I still don't grok everything. In particular I don't understand the function of all the "that"/"foo" lines I mentioned in #501:
Image

I'm looking in the great news docs but I'm not sure if this is missing or I'm just not seeing it. I can see that the pattern.add()s are adding two casings of a word to "A pattern collection to look for patterns that start with a specific word." with "A pattern that checks that a sequence of other patterns match." as the other argument ... but I don't know what that means.

By the way, I have more questions in my other PR for a real lint: #502

I'm also stuck on another real lint I've started. I could also do a work-in-progress PR for that one if you like in the hope that me asking all the dumb questions now will make it so future contributors won't have to ask them again (-:

@hippietrail
Copy link
Copy Markdown
Collaborator Author

OK nice. What about something like a --verbose switch that will show all matching rules and not just the first?

I've set that up. You can either use just lint-verbose <file> or cargo run --bin harper-cli -- <file> --verbose

That's great! But now I can see that with or without --verbose it only shows the Lint.messages but doesn't show the suggestions. Should it do both or should that be a separate option? I haven't found another way to test suggestions from the commandline so please let me know if there's a way that I've missed.

@elijah-potter
Copy link
Copy Markdown
Collaborator

I haven't found another way to test suggestions from the commandline so please let me know if there's a way that I've missed.

Honestly, the best way to test suggestions is in an actual editor (like Visual Studio Code). Have you tried running your own build in there?

@hippietrail
Copy link
Copy Markdown
Collaborator Author

hippietrail commented Jan 28, 2025

I haven't found another way to test suggestions from the commandline so please let me know if there's a way that I've missed.

Honestly, the best way to test suggestions is in an actual editor (like Visual Studio Code). Have you tried running your own build in there?

I haven't figured out how to do that.
I spoke to soon. I think I've got it set up now...

@hippietrail hippietrail closed this May 6, 2025
@hippietrail hippietrail deleted the example_new_rule branch May 6, 2025 00:01
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