Skip to content

New api #195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Apr 16, 2025
Merged

New api #195

merged 22 commits into from
Apr 16, 2025

Conversation

Muscraft
Copy link
Member

This PR moves annotate-snippets internals to better match rustc, better matches rustc's output, adds suggestions, and moves to a new API. The new API is subject to change as user feedback comes in on how it can be improved.

Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've discussed this offline and with the branch.

I expect we'll iterate further within the repo, rather than keep this massive change in the dark further

Copy link
Contributor

@zbraniecki zbraniecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this reads great! Thank you for driving the update.
I'm surprised how little the code changes and I like the Group semantics.

I checked the benchmarks between today's master:

❯ cargo criterion
    Finished `bench` profile [optimized + debuginfo] target(s) in 0.08s

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Timer precision: 41 ns
bench         fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ fold                     │               │               │               │         │
│  ├─ 0       3.915 µs      │ 41.49 µs      │ 4.041 µs      │ 4.507 µs      │ 100     │ 100
│  ├─ 1       3.874 µs      │ 4.79 µs       │ 3.999 µs      │ 4.046 µs      │ 100     │ 100
│  ├─ 10      3.999 µs      │ 5.749 µs      │ 4.124 µs      │ 4.16 µs       │ 100     │ 100
│  ├─ 100     5.165 µs      │ 6.833 µs      │ 5.249 µs      │ 5.296 µs      │ 100     │ 100
│  ├─ 1000    16.45 µs      │ 94.54 µs      │ 16.58 µs      │ 18.01 µs      │ 100     │ 100
│  ├─ 10000   129.6 µs      │ 182.2 µs      │ 133.7 µs      │ 137.8 µs      │ 100     │ 100
│  ╰─ 100000  1.289 ms      │ 1.829 ms      │ 1.413 ms      │ 1.443 ms      │ 100     │ 100
╰─ simple     28.79 µs      │ 45.54 µs      │ 29.08 µs      │ 29.66 µs      │ 100     │ 100

and your PR:

❯ cargo criterion
    Finished `bench` profile [optimized + debuginfo] target(s) in 0.08s

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Timer precision: 41 ns
bench         fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ fold                     │               │               │               │         │
│  ├─ 0       4.041 µs      │ 46.12 µs      │ 4.166 µs      │ 4.616 µs      │ 100     │ 100
│  ├─ 1       3.999 µs      │ 5.207 µs      │ 4.166 µs      │ 4.195 µs      │ 100     │ 100
│  ├─ 10      4.124 µs      │ 5.832 µs      │ 4.291 µs      │ 4.324 µs      │ 100     │ 100
│  ├─ 100     5.457 µs      │ 7.374 µs      │ 5.583 µs      │ 5.629 µs      │ 100     │ 100
│  ├─ 1000    17.41 µs      │ 19.37 µs      │ 17.58 µs      │ 17.62 µs      │ 100     │ 100
│  ├─ 10000   132.5 µs      │ 253.4 µs      │ 137.8 µs      │ 141.3 µs      │ 100     │ 100
│  ╰─ 100000  1.295 ms      │ 1.658 ms      │ 1.444 ms      │ 1.441 ms      │ 100     │ 100
╰─ simple     29.62 µs      │ 49.2 µs       │ 29.95 µs      │ 30.32 µs      │ 100     │ 100

Seems all good!

@Muscraft Muscraft deleted the new-api branch April 16, 2025 21:05
@epage epage mentioned this pull request Apr 16, 2025
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