merlin document support for PPXs #167
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liam923
requested changes
Jul 21, 2025
Contributor
liam923
left a comment
There was a problem hiding this comment.
Overall this looks good! I think the main things are just adding more tests and moving some more of the logic into ppx_document.ml.
liam923
requested changes
Jul 22, 2025
liam923
requested changes
Jul 23, 2025
Contributor
liam923
left a comment
There was a problem hiding this comment.
Awesome, pretty much just one nit left.
liam923
reviewed
Jul 23, 2025
liam923
approved these changes
Jul 24, 2025
Contributor
There was a problem hiding this comment.
Awesome, thanks for these changes! Feel free to merge (I think you have permission to hit the merge button?). Sidenote: in oxcaml/merlin we tend to prefer "Squash and merge" over "Create a merge commit". I think this makes the git history a lot more managable. The exception to this is when we're merging changes from upstream, in which case this can confuse git and create future merges harder.
liam923
pushed a commit
that referenced
this pull request
Jul 30, 2025
* initial working with tests * temp update * temp * major refactors, add support for .mli files * refactoring and cleanup * move all logic into override_document.ml; cleanup code; add logging * add many more tests; handle edge cases * revert std.ml changes * improve comments * update more comments * suggested changes * test nested ppx * document payload test * some more tests * resolve final comments * add test on floating attribute * add test for attribute in an extension's payload
liam923
added a commit
that referenced
this pull request
Aug 7, 2025
* merlin document support for PPXs (#167) * initial working with tests * temp update * temp * major refactors, add support for .mli files * refactoring and cleanup * move all logic into override_document.ml; cleanup code; add logging * add many more tests; handle edge cases * revert std.ml changes * improve comments * update more comments * suggested changes * test nested ppx * document payload test * some more tests * resolve final comments * add test on floating attribute * add test for attribute in an extension's payload * Fix function syntax * working parsing of new merlin.document structure * fix document tests with new merlin.document structure * replicate merlin.document tests for merlin.locate * generalize comments, code for all overrides * delete duplicate files from merge conflict * add test overriding locate of a non-PPX * add test on overriding default behavior of document * format * combine document and locate tests into single file * adjust for new attribute structure * update comment documenting expected attribute structure * fix typo * remove named parameter * merlin locate outputs absolute paths using source_root --------- Co-authored-by: Liam Stevenson <lstevenson@janestreet.com>
liam923
added a commit
that referenced
this pull request
Aug 15, 2025
* merlin document support for PPXs (#167) * initial working with tests * temp update * temp * major refactors, add support for .mli files * refactoring and cleanup * move all logic into override_document.ml; cleanup code; add logging * add many more tests; handle edge cases * revert std.ml changes * improve comments * update more comments * suggested changes * test nested ppx * document payload test * some more tests * resolve final comments * add test on floating attribute * add test for attribute in an extension's payload * Fix function syntax * merlin overrides: merlin locate support for PPXs (#171) * merlin document support for PPXs (#167) * initial working with tests * temp update * temp * major refactors, add support for .mli files * refactoring and cleanup * move all logic into override_document.ml; cleanup code; add logging * add many more tests; handle edge cases * revert std.ml changes * improve comments * update more comments * suggested changes * test nested ppx * document payload test * some more tests * resolve final comments * add test on floating attribute * add test for attribute in an extension's payload * Fix function syntax * working parsing of new merlin.document structure * fix document tests with new merlin.document structure * replicate merlin.document tests for merlin.locate * generalize comments, code for all overrides * delete duplicate files from merge conflict * add test overriding locate of a non-PPX * add test on overriding default behavior of document * format * combine document and locate tests into single file * adjust for new attribute structure * update comment documenting expected attribute structure * fix typo * remove named parameter * merlin locate outputs absolute paths using source_root --------- Co-authored-by: Liam Stevenson <lstevenson@janestreet.com> * merlin overrides use interval tree instead of list (#173) * interval tree interface * interval tree implementation and basic test * use array for construction, add more tests * add and update comments * fix error messages, validate inputs * use list instead of array, add tests, add comments * integrate into overrides.ml * add test for empty tree * resolve requested changes * add test for zero-width interval * undo test reformat * update outdated comment * update another oudated comment * update comment * Cache overrides interval trees using Phase_cache (#176) * implementation but failing tests * working overrides caching with tests * add USE_PPX_CACHE, add all cache entries to tests * update comments * use Option.map * move comment * fix overrides caching mechanism, functorize caches, add cache invalidation test * add comments, rename * cleanup test output, and resolve other comments * remove unnecessary sed commands from tests * add commentsg --------- Co-authored-by: Max Wang <orangemax888@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds upon features written in our internal codebase.
[@@@merlin.document]attributeTesting
Adds unit tests for
.mland.mlifiles