Skip to content

feat(core): start support for hex numbers#553

Merged
elijah-potter merged 5 commits intoAutomattic:masterfrom
hippietrail:hex-543
Feb 5, 2025
Merged

feat(core): start support for hex numbers#553
elijah-potter merged 5 commits intoAutomattic:masterfrom
hippietrail:hex-543

Conversation

@hippietrail
Copy link
Copy Markdown
Collaborator

towards #543

based on the existing lex_number which cuts off when the digits finish, which is fine because a unit of measurement etc might immediately follow. But with a hex number a non-hex letter might follow and not be flagged if it's a single letter. Also if it's very wrong, the start won't be flagged if it's 0x and at least one hex digit. Examples will help illustrate:

image

So just a draft PR to solicit thoughts and mods.

@hippietrail hippietrail marked this pull request as ready for review February 1, 2025 05:34
@hippietrail
Copy link
Copy Markdown
Collaborator Author

I'm still figuring out GitHub. I made this PR a draft because it's not ready to be merged.

But I think "draft" also means "not ready for review" but I specifically want it reviewed to get feedback on what it should do with text "stuck to" the hex that is not hex. And perhaps how to change the code to handle comply to what's decided. Especially if we want to flag such "hex with appendages".

If my understanding of draft status for a PR is still wrong, please let me know.

Copy link
Copy Markdown
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.

@hippietrail, the code looks good. Would you mind adding some test cases under harper-core/tests/test_sources?

I would add a couple Markdown files that should be considered "clean" with this lexer added. Make sure you actually import them in harper-core/tests/run_tests.rs.

@hippietrail
Copy link
Copy Markdown
Collaborator Author

hippietrail commented Feb 1, 2025

@hippietrail, the code looks good. Would you mind adding some test cases under harper-core/tests/test_sources?

I would add a couple Markdown files that should be considered "clean" with this lexer added. Make sure you actually import them in harper-core/tests/run_tests.rs.

Are you sure? Did you see my concerns about what to do regarding non-hex text immediately after the hex with no space etc between?


After some thought I came to grok the problem more completely and refactored the code, now written in a more old-school way that I'm used to doing lexing which makes it easier to think about when it gets tricky.

I added positive and negative tests each in a loop. Let me know if this is a good or bad idea.

Clear behaviour on non-hex appended to valid hex.
Added test cases.

Note that the `lex_number` will match the `0` of hex with appended non-hex though. Sould be vary rare but would be counterinutive for the user.
Comment thread harper-core/src/lexing/mod.rs Outdated
@ccoVeille
Copy link
Copy Markdown
Contributor

I'm worried about possible reporting of leet speak like 0xf0rd

Then it's a matter of balance, what is the more likely to be found in text? Hexadecimal or leet speak ?

I updated #555 accordingly.

I would prefer hex rules to be merged after leet speak is handled #598

@elijah-potter elijah-potter merged commit 0674751 into Automattic:master Feb 5, 2025
@ccoVeille
Copy link
Copy Markdown
Contributor

I'm worried about possible reporting of leet speak like 0xf0rd

Then it's a matter of balance, what is the more likely to be found in text? Hexadecimal or leet speak ?

I updated #555 accordingly.

I would prefer hex rules to be merged after leet speak is handled #598

@elijah-potter you merged. It's OK, but can I get a feedback on what I explained?

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=-->
@hippietrail hippietrail deleted the hex-543 branch April 3, 2025 18:37
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.

3 participants