Skip to content

feat(core): Lint for those who don't like Oxford commas#562

Merged
elijah-potter merged 1 commit intoAutomattic:masterfrom
hippietrail:no-oxford-comma-lint
Feb 4, 2025
Merged

feat(core): Lint for those who don't like Oxford commas#562
elijah-potter merged 1 commit intoAutomattic:masterfrom
hippietrail:no-oxford-comma-lint

Conversation

@hippietrail
Copy link
Copy Markdown
Collaborator

I do use them, but many dislike them.
As I noted on issue #355 (comment)

I do use them, but many dislike them.
As noted in my comment on Automattic#355 (comment)
@hippietrail
Copy link
Copy Markdown
Collaborator Author

Since there is not yet any mutual-exclusive switch for opposing preferences, both "Oxford comma" and "No Oxford comma" can be active at once. A side-effect is any lists not flagged for either probably indicate some imperfection in the sentence parsing.

@elijah-potter
Copy link
Copy Markdown
Collaborator

I'm quite tempted to merge this. Once I do, @hippietrail would you mind using Harper with this linter enabled for a few days to catch any issues? If you find any, we can open a new PR with fixes.

@hippietrail
Copy link
Copy Markdown
Collaborator Author

I'm quite tempted to merge this. Once I do, @hippietrail would you mind using Harper with this linter enabled for a few days to catch any issues? If you find any, we can open a new PR with fixes.

Absolutely! You may notice I filed a number of issues with other linters yesterday.

@elijah-potter
Copy link
Copy Markdown
Collaborator

Fantastic. Merging...

@elijah-potter elijah-potter merged commit b40715d into Automattic:master Feb 4, 2025
@hippietrail hippietrail deleted the no-oxford-comma-lint branch February 4, 2025 15:37
@hippietrail
Copy link
Copy Markdown
Collaborator Author

Fantastic. Merging...

Try as I might I cannot get it to work now in VS Code or Windsurf. It does work with harper-cli though.
Could it be related to there not being an option in the settings for NoOxfordComma?

???

As I came to write this it has now flipped and it's only flagging the sentences with Oxford commas and not the ones without. It's almost 1am so maybe I'm just missing something. I'm going to sleep on it but would be great if somebody could test on their system.

@mcecode
Copy link
Copy Markdown
Collaborator

mcecode commented Feb 5, 2025

Try as I might I cannot get it to work now in VS Code or Windsurf. It does work with harper-cli though. Could it be related to there not being an option in the settings for NoOxfordComma?

It should still work as long as the default setting for NoOxfordComma is true in your harper-ls build. If you'd like there to be a setting for it within VSCode when you're trying it out in the extension host, you can run just update-vscode-linters to add any missing linters that's in the current master build but not in the latest release before opening the extension host.

@hippietrail
Copy link
Copy Markdown
Collaborator Author

just update-vscode-linters

I just committed what I was working on to go back to master to see if it's still happening. And I now have two pinned cores. just update-vscode-linters and reloading the window did not make both Oxford lints work again.

I wonder if switching branches with harper-cli running is causing the pinned cores, and that in turn might be causing the old harper-cli to still be running instead of the updated one with both the "Oxford" and "no Oxford" lints?

@mcecode
Copy link
Copy Markdown
Collaborator

mcecode commented Feb 7, 2025

Sorry it took me a while to get back to you on this.

I just committed what I was working on to go back to master to see if it's still happening. And I now have two pinned cores. just update-vscode-linters and reloading the window did not make both Oxford lints work again.

just update-vscode-linters is only meant to allow you to set the new NoOxfordComma rule within VSCode. Did the Oxford comma lints not work despite setting both of them to true within VSCode?

I wonder if switching branches with harper-cli running is causing the pinned cores, and that in turn might be causing the old harper-cli to still be running instead of the updated one with both the "Oxford" and "no Oxford" lints?

So are you having problems with harper-cli now or did you mean harper-ls when you said harper-cli? If it's the latter, yes, changing branches and running different/old versions of harper-ls may be what's causing the Oxford comma lints to not work.

@hippietrail
Copy link
Copy Markdown
Collaborator Author

Sorry it took me a while to get back to you on this.

I just committed what I was working on to go back to master to see if it's still happening. And I now have two pinned cores. just update-vscode-linters and reloading the window did not make both Oxford lints work again.

just update-vscode-linters is only meant to allow you to set the new NoOxfordComma rule within VSCode. Did the Oxford comma lints not work despite setting both of them to true within VSCode?

I wonder if switching branches with harper-cli running is causing the pinned cores, and that in turn might be causing the old harper-cli to still be running instead of the updated one with both the "Oxford" and "no Oxford" lints?

So are you having problems with harper-cli now or did you mean harper-ls when you said harper-cli? If it's the latter, yes, changing branches and running different/old versions of harper-ls may be what's causing the Oxford comma lints to not work.

harper-cli always worked for both OxfordComma and NoOxfordComma. But harper-ls Would only work with one until suddenly it switched to only working with the other. But now they are both working. It seemed odd that killing the harper-ls processes after changing branches didn't fix. I may have mixed them up when I was at my most frustrated and confused.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Feb 11, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [Automattic/harper/harper-ls](https://github.com/Automattic/harper) | minor | `v0.19.1` -> `v0.20.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>Automattic/harper (Automattic/harper/harper-ls)</summary>

### [`v0.20.0`](https://github.com/Automattic/harper/releases/tag/v0.20.0)

[Compare Source](Automattic/harper@v0.19.1...v0.20.0)

#### What's Changed

-   chore: Maintenance Changes by [@&#8203;mcecode](https://github.com/mcecode) in Automattic/harper#583
-   build: use `just`-native dependency resolution by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#554
-   build(deps): bump tree-sitter-dart from 0.0.3 to 0.0.4 by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#566
-   build(deps): bump serde_json from 1.0.137 to 1.0.138 by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#568
-   build(deps-dev): bump [@&#8203;rollup/plugin-typescript](https://github.com/rollup/plugin-typescript) from 11.1.6 to 12.1.2 in /packages by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#569
-   feat(web): add link to the Discord server by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#585
-   clean up strange character at end-of-line by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#580
-   feat(core): Lint for those who don't like Oxford commas by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#562
-   docs(obsidian): notify people they should use up-to-date Electron by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#586
-   build(deps-dev): bump vitest from 2.1.8 to 2.1.9 in /packages by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#588
-   build(deps-dev): bump [@&#8203;vitest/browser](https://github.com/vitest/browser) from 2.1.8 to 2.1.9 in /packages by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#591
-   fix(core): add `on` to the list of special cases by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#592
-   feat(core): start support for hex numbers by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#553
-   feat: change `just addnoun` to have different affixes as per [#&#8203;595](Automattic/harper#595) by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#601
-   Phrase fixes by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#589
-   dict: add inclusivity by [@&#8203;lukasmwerner](https://github.com/lukasmwerner) in Automattic/harper#590
-   docs(vscode): revised introductional material by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#602
-   docs: missing word in web/author-a-rule by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#614
-   feat: adds a brief helpful comment to each entry describing its function by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#593
-   fix(ls): release config lock to avoid deadlocks by [@&#8203;nyonson](https://github.com/nyonson) in Automattic/harper#612
-   feat(core): create rule for the possessive version of "you" by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#619
-   bench(harper.js): add benchmarks for configuration methods by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#618
-   grammar agreement in obsidion plugin comment by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#622
-   feat: clarify message for uncapitalized "I" by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#625
-   build(deps-dev): bump [@&#8203;types/node](https://github.com/types/node) from 22.13.0 to 22.13.1 in /packages by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#632
-   build(deps-dev): bump esbuild from 0.24.2 to 0.25.0 in /packages by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#633
-   build(deps-dev): bump typescript from 5.6.3 to 5.7.3 in /packages by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#631
-   build(deps-dev): bump [@&#8203;sveltejs/kit](https://github.com/sveltejs/kit) from 2.16.1 to 2.17.1 in /packages by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#629
-   chore: correct some affix annotations by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#617
-   feat: split triggers in `matcher.rs` into categories with explanations by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#636
-   Cut-and-paste error in "report error" template by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#623
-   bug: reverse correcting "supposed to" to "suppose to" by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#621
-   feat: if spellcheck only has one suggestion mention it directly by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#626
-   fix(obsidian): correctly handle Unicode conversions by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#627
-   feat: add `harper-cli forms` and `just getforms` by [@&#8203;hippietrail](https://github.com/hippietrail) in Automattic/harper#615
-   Closed Compound Matcher Conversions by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#608
-   build(deps): bump clap from 4.5.27 to 4.5.28 by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#635
-   build(deps): bump esbuild from 0.23.1 to 0.25.0 in /packages by [@&#8203;dependabot](https://github.com/dependabot) in Automattic/harper#639
-   fix(core): make narrow linter specifically for `long and behold` by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#641
-   fix(core): currency placement with certain decimal positions by [@&#8203;elijah-potter](https://github.com/elijah-potter) in Automattic/harper#604

#### New Contributors

-   [@&#8203;nyonson](https://github.com/nyonson) made their first contribution in Automattic/harper#612

**Full Changelog**: Automattic/harper@v0.19.1...v0.20.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xNjQuMSIsInVwZGF0ZWRJblZlciI6IjM5LjE2NC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
@NicholasFlamy
Copy link
Copy Markdown

In VSCodium, it doesn't tell me to remove it with this option disable, but it does tell me to add it with this option enabled. Luckily I'm for Oxford commas, so I'm happy.

@hippietrail
Copy link
Copy Markdown
Collaborator Author

In VSCodium, it doesn't tell me to remove it with this option disable, but it does tell me to add it with this option enabled. Luckily I'm for Oxford commas, so I'm happy.

Until now I only had one of the Oxford comma options show up, but now I see both at last.
Though now I see some other problems... will report...

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.

4 participants