Add check-spelling #535
Add check-spelling #535
Conversation
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
| uses: actions/checkout@v2.0.0 | ||
| with: | ||
| fetch-depth: 5 | ||
| - uses: check-spelling/check-spelling@prerelease |
There was a problem hiding this comment.
I intend to change this to (once it's released):
| - uses: check-spelling/check-spelling@prerelease | |
| - uses: check-spelling/check-spelling@0.0.18-alpha |
| with: | ||
| experimental_apply_changes_via_bot: 1 |
There was a problem hiding this comment.
This feature is experimental (as the name indicates) with 0.0.18-alpha being the first release to include it. The benefit is that instead of users manually maintaining the expect.txt file (or running code which it suggests), the bot can do it for them. It's a tradeoff. I'm not sure what a given project would want to do.
There was a problem hiding this comment.
I like the idea of less hassle. Can this be done only for PRs, though?
There was a problem hiding this comment.
This feature (in preview, and 0.0.18 once released) is only available for PRs. Specifically while GitHub does have a comment event, it doesn't get sent to GitHub workflows, so the only chances I have to respond to comments are issue_comment (which practically means a PR comment, since the bot will not respond to a thing that doesn't have a proper comment from itself and a branch...) and pull_request_review_comment (which I am not handling now -- and since the bot's comment is top level, it doesn't make sense for a user to somehow reply to the bot in the wrong place).
I am planning to offer a way for repositories / commits to ask the bot to do the work.
Something like a commit message:
feat: Add support for blah-parsing
@check-spelling-bot: update expect
Which would then be handled by the push handler. I hope to be able to do this for 0.0.19. The precise way this will work is still entirely in the air. As noted elsewhere, I'm still working on the wiring to support it, and I haven't even started thinking about the command structure. (In a last minute thing for 0.0.18, I added support for the bot also updating the excludes.txt file in addition to expect.txt.)
| on: | ||
| pull_request_target: | ||
| push: | ||
| issue_comment: |
There was a problem hiding this comment.
This event works with experimental_apply_changes_via_bot
| if: "contains(github.event_name, 'pull_request')" | ||
| uses: actions/checkout@v2.0.0 | ||
| with: | ||
| ref: refs/pull/${{github.event.pull_request.number}}/merge |
There was a problem hiding this comment.
This conditional magic handles the distinction between how pull_request_target and pull_request work. It's a delicate balance.
There was a problem hiding this comment.
It will always add a commit on top, right? Why not use /head instead of /merge, then?
There was a problem hiding this comment.
The fixup is to /head yes. The main reason is if someone manages to add a typo to main such that main was ❌ when someone branched off, if they then make a PR, they get a confusing state for something and try to fix it (or have no idea what to do/why things are broken). It's mostly to show a possible workflow. I'm totally fine w/ using /head.
| name: Spell checking | ||
| on: | ||
| pull_request_target: | ||
| push: |
There was a problem hiding this comment.
Having push means that users can get feedback on misspellings before they make PRs -- if they've enabled actions in their forks (which they don't have to do).
There was a problem hiding this comment.
Right. I think this is a good idea.
| ccd | ||
| CDcd | ||
| CEu | ||
| charset |
There was a problem hiding this comment.
I suspect a number of the words aren't in cSpell because they're pulled in by its optional dictionaries. Which I think I'll be able to use in 0.0.19...
| CIsmtp | ||
| CIsmtphost | ||
| CIsmtpopts | ||
| CIsmtppass |
There was a problem hiding this comment.
this set should look familiar
There was a problem hiding this comment.
I really don't like that there are two lists with essentially the same content... :-(
There was a problem hiding this comment.
Yeah, I get it.
Adding a plug-in for VSCode is on the list of things I might do: https://github.com/check-spelling/check-spelling/wiki/Feature%3A-Visual-Studio-Code-plugin, but it's quite a ways down and I haven't started thinking through how to actually do it.
Fwiw, in this case, cSpell is set to think about these items as regular expressions. I have no idea why someone chose to do that. You can see that I'm just treating them as word-like terms.
There was a problem hiding this comment.
For the record, I have no intention replacing cSpell in my VS Code installation with any other spell checker. I would want to use the same word list, not another spell checker.
| destructuring | ||
| devnull | ||
| devops | ||
| DFEEDBEEF |
There was a problem hiding this comment.
It's possible to add more patterns to reduce the amount of things like this. I stopped when it got to a manageable level.
| gitattributes | ||
| gitconfig | ||
| gitgit | ||
| gitgitgadget |
There was a problem hiding this comment.
I would normally move this into allow.txt (a supplemental dictionary), but hadn't reached that stage yet.
| @@ -0,0 +1,27 @@ | |||
| <!-- See https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples%3A-advice --> <!-- markdownlint-disable MD033 MD041 --> | |||
| <details><summary>If you see a bunch of garbage</summary> | |||
There was a problem hiding this comment.
This content is included as supplemental text to a GitHub comment when a commit/PR introduces unknown words into the repository in a file that isn't excluded on a line where that content isn't masked.
|
Oh wow, I completely forgot about this PR, sorry. What's the best way forward from here? |
|
@jsoref are you still interested in this PR? |
|
Yeah, still interested. I'll try to look tonight. |
|
@jsoref what was the result of that? |
|
Sorry. Lemme get back to you in the back part of the month. I want to do one more release with a number of improvements. |
This PR would add the check-spelling action (as used for #529).
Per #529 (comment), I'm leaving this as a draft.
I'm about to release 0.0.18-alpha and don't want to carry some extra two letter words from 0.0.17-alpha that 0.0.18-alpha will obsolete.
This will also give the project space to talk about/look over how it would want things.
I've been including a readme with my configuration to make it easier for people to understand how to maintain the files that control it, the sample one is here:
https://github.com/gitgitgadget/gitgitgadget/blob/3e93eb0aa6f178df3f7b44d7adb734c4289b4bb1/.github/actions/spelling/README.md (based on https://github.com/check-spelling/spell-check-this/blob/main/.github/actions/spelling/README.md). @dscho suggested putting such content in the commit message instead, and I'm totally fine with that.
The documentation for check-spelling is in the wiki: https://github.com/check-spelling/check-spelling/wiki