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.

Sign up for GitHub

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

Jump to bottom

fix(completion): correctly parse incomplete attrsets and provide completions #644

Merged
merged 2 commits into from
Jan 15, 2025
Merged

fix(completion): correctly parse incomplete attrsets and provide completions #644

merged 2 commits into from
Jan 15, 2025

Conversation

Aetherall
Copy link
Contributor

Small DX adjustments:

  • Parser distinguishes a little better formals from attrsets
  • Completion are triggered when discovering options of an attrset ( no need to try and guess the first letter )

I had to remove a check for the latter, please review carefully !

@Aetherall Aetherall requested a review from inclyc as a code owner January 13, 2025 13:30
@Aetherall Aetherall force-pushed the fix/completions branch 2 times, most recently from 8f44958 to 2cedd1a Compare January 13, 2025 13:52
inclyc
inclyc approved these changes Jan 14, 2025
View reviewed changes
Copy link
Member

@inclyc inclyc left a comment

Choose a reason for hiding this comment

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

This PR looks great to me, and it’s an impressive improvement to the user experience. I just have a few minor suggestions, mostly regarding coding style. Thank you!

libnixf/include/nixf/Basic/Range.h Outdated Show resolved Hide resolved
libnixf/include/nixf/Basic/Range.h Outdated Show resolved Hide resolved
nixd/lib/Controller/Completion.cpp Outdated Show resolved Hide resolved
@Aetherall
Copy link
Contributor Author

Thanks for the review !

I would really appreciate also if you could try it out locally, because maybe there are some edge cases not tested I missed out entirely. I never worked on a lexer / parser before.

I will fix the comments after work (3-4h from now in my TZ)

@inclyc inclyc linked an issue Jan 15, 2025 that may be closed by this pull request
Autocompletion does not work on the first line a configuration object #539
Closed
inclyc
inclyc approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@inclyc inclyc left a comment

Choose a reason for hiding this comment

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

LGTM! I believe this is ready to be merged.

@inclyc inclyc merged commit 2c25600 into nix-community:main Jan 15, 2025
19 checks passed
@inclyc inclyc added the bug Something isn't working label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@inclyc inclyc inclyc approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Autocompletion does not work on the first line a configuration object
2 participants
@Aetherall @inclyc