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

Create and install a test_lib.cmi file #205

Closed
yurug opened this issue Dec 6, 2018 · 6 comments · Fixed by #481
Closed

Create and install a test_lib.cmi file #205

yurug opened this issue Dec 6, 2018 · 6 comments · Fixed by #481
Labels
kind: enhancement Enhancement to an existing user-facing feature.

Comments

@yurug
Copy link
Collaborator

yurug commented Dec 6, 2018

When we develop graders, it would be convenient to have .merlin know the signature of Test_lib.

We should probably compile Test_lib interface and install the resulting cmi file in the learn-ocaml package directory.

@erikmd
Copy link
Member

erikmd commented Dec 6, 2018

@yurug indeed, Merlin is currently not aware of the signature of Test_lib, but I'm rather unsure it would be feasible to implement it using .merlin, because AFAICT test.ml is not a standalone file to be compiled, and Test_lib is not a regular compilation unit: it is the result of the module instantiation performed at runtime:

Toploop_ext.use_string ~print_outcome ~ppf_answer
"module Test_lib = Test_lib.Make(struct\n\
\ let results = results\n\
\ let set_progress = set_progress\n\
\ let timeout = timeout\n\
\ module Introspection = Introspection\n\
end)" ;

However, note that we were able to implement one similar feature in the learn-ocaml-editor project: when the teacher clicks on the Check button of the Test.ml tab, the test.ml file is prepended with a "mock" instantiation module and sent to the toploop (along with prelude.ml and prepare.ml), so the teacher can typecheck the test.ml code this way, and in case of error the editor webapp subsequently recomputes the proper line numbers to highlight the error at stake (basically offset subtraction), or switch to the Prelude/Prepare tabs if the error lies there.

@erikmd
Copy link
Member

erikmd commented Dec 6, 2018

FYI this feature can be tested in the live demo of learn-ocaml-editor (currently version 0.4.0) here:
https://pfitaxel.github.io/pfitaxel-demo/
and as an accompanying documentation there is a stepwise tutorial here:
https://github.com/pfitaxel/learn-ocaml-editor/wiki/Demos-2018-W28
(although this wiki page is now a bit old as it dates back to mid-July)

@yurug
Copy link
Collaborator Author

yurug commented Dec 6, 2018

Thank you @erikmd! I am aware of this dynamic creation of Test_lib for the grading toplevel. However, I do not understand why we cannot compile a fake .mli file for the sole purpose of informing merlin about what can be expected in Test_lib.

@erikmd
Copy link
Member

erikmd commented Dec 6, 2018

Hi @yurug,

I do not understand why we cannot compile a fake .mli file for the sole purpose of informing merlin about what can be expected in Test_lib.

OK I see, this could be feasible indeed :) but it seems to me this solution couldn't be "complete", because in practice test.ml can rely on other functions/expressions not in Test_lib, including:

  • set_progress, code_ast (this can admittedly be workarounded)
  • and unqualified functions from Prelude and Prepare…

Kind regards, Erik

@fpottier
Copy link
Contributor

Hear, hear! I was just about to ask for this feature.

@fpottier
Copy link
Contributor

Regarding functions in Prelude and Prepare, is it permitted for the code in test.ml to refer to these functions by their full (qualified) names? That would allow Merlin to work as desired, I think.

@AltGr AltGr added the kind: enhancement Enhancement to an existing user-facing feature. label Jan 22, 2019
@AltGr AltGr closed this as completed in #481 Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Enhancement to an existing user-facing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants