Skip to content
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

Refactor enum_literal compls #1493

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

llogick
Copy link
Contributor

@llogick llogick commented Oct 2, 2023

Not perfect, but functional .

@llogick llogick force-pushed the nullptrdevs/refactor-enum_literal_compls branch from 7f30056 to 4b52065 Compare October 5, 2023 22:39
@llogick llogick requested a review from leecannon October 6, 2023 00:36
@llogick llogick force-pushed the nullptrdevs/refactor-enum_literal_compls branch from 4b52065 to cdf237a Compare October 6, 2023 20:34
@llogick llogick linked an issue Oct 7, 2023 that may be closed by this pull request
Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

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

I did not perform any more checks on my suggestions than running tests so they may break something.
The only exception is the suggestion about the @import builtin resolution since the current testing infrastructure doesn't support multi file completion tests.

Could you also please remove your debugging related print statements including those that you have comment out.

src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
@llogick llogick force-pushed the nullptrdevs/refactor-enum_literal_compls branch from cdf237a to 55eb9e8 Compare October 10, 2023 00:40
@llogick
Copy link
Contributor Author

llogick commented Oct 10, 2023

Applied all of the above, multi file completions seem to work.

Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

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

second round of refactoring and simplification. tests still pass...

src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
src/features/completions.zig Outdated Show resolved Hide resolved
Co-authored-by: Techarix <19954306+Techatrix@users.noreply.github.com>
@llogick llogick force-pushed the nullptrdevs/refactor-enum_literal_compls branch from 55eb9e8 to 102f1af Compare October 10, 2023 04:26
@llogick
Copy link
Contributor Author

llogick commented Oct 10, 2023

Ditto

@Techatrix
Copy link
Member

LGTM, hopefully nothing has regressed through these changes.
I will run some fuzzing overnight so that we don't introduce any new crashes.

Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

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

Fuzzing only reported a single crash which was unrelated to this PR. (InternPool related)

@llogick llogick merged commit 61fec01 into master Oct 11, 2023
@llogick llogick deleted the nullptrdevs/refactor-enum_literal_compls branch October 11, 2023 03:03
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.

Incorrect enum autocomplete results in switch expression
2 participants