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

(RDY) Parse correct locations extension expressions sugar #2162

Merged
merged 1 commit into from
Aug 30, 2018

Conversation

IwanKaramazow
Copy link
Contributor

Fixes #2143

@jordwalke
Copy link
Member

You are awesome. Did you test via linking a local build of Reason?

@andreypopp
Copy link
Contributor

I did test it on https://github.com/andreypopp/reason-ppx-let-merlin and seeing the same behaviour as previously. Did I missing something? refmt and ocamlmerlin-reason point to local builds.

@IwanKaramazow
Copy link
Contributor Author

@jordwalke, using esy link to test this out locally
Still WIP, the locations under extension sugar need to be revised...

Needed for Merlin's jump to location and type info in editor.
@IwanKaramazow IwanKaramazow changed the title (WIP) Fix behaviour Merlin under Pexp_extensions (RDY) Fix behaviour Merlin under Pexp_extensions Aug 30, 2018
@IwanKaramazow IwanKaramazow changed the title (RDY) Fix behaviour Merlin under Pexp_extensions (RDY) Parse correct locations extension expressions sugar Aug 30, 2018
@IwanKaramazow
Copy link
Contributor Author

@jordwalke this is ready.

Locations where missing in parts of the extension expression, resulting in Merlin have a very bad time.
Fixed all locations and streamlined the behaviour in terms of what the stock Ocaml parser does.

image

@andreypopp
Copy link
Contributor

Just tested this and it works great! Autocomplete also started working for me (previously bindings introduced by let%bind weren't visible). Thank you @IwanKaramazow!

@jordwalke
Copy link
Member

🎉
Thanks, this was the main blocker for converting more of the esy codebase (parts that use async operations)
Thank you, as always.

@jordwalke jordwalke merged commit 335c3fb into reasonml:master Aug 30, 2018
@zindel
Copy link

zindel commented Sep 2, 2018

This works like a charm within the fastpack codebase! Thank you so much, @IwanKaramazow, this makes a huge difference!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants