Integrate learn-ocaml-editor #295

erikmd · 2019-07-05T18:05:40Z

Kind: feature
Depends: #302

Dear all,

This PR adds the "learn-ocaml-editor" feature that has been developed in https://github.com/pfitaxel/learn-ocaml-editor by the PFITAXEL team, recently migrated by @leunam217 to the latest version of learn-ocaml (18f0ddc) then simplified − removing one view (high-level version of "Test") and renaming "Test.ml" to "Test" − to anticipate an upcoming evolution to be implemented within @leunam217's internship co-supervised by @yurug and me.

Brief overview of the design choices

From the beginning of the development of this feature, the idea is to provide a local web environment (fully client-side & statically served) for the teachers to create learn-ocaml exercises with:

a facility to edit all the components of an exercise in the "exodir format" (viz. solution.ml, template.ml, prepare.ml, prelude.ml, test.ml, metadata etc.) with a faster development cycle for teachers.
in particular:
- some "Check" buttons are available in all the corresponding tabs
  ~~(albeit since the migration an issue has to be fixed with the "Test" tab)~~ [now fixed]
- a "Generate" feature allows one to create monomorphic test cases automatically from the solution, as well as a "Quality Code" section that we had devised for our teaching unit "PFITA"
- a "Grade" feature allows one to grade the solution w.r.t. the test suite
- and an "Experiment" feature (which has been temporarily disabled due to migration concerns) that will allow teachers to immediately visualize the devised exercise from a student point of view (in particular, learn-ocaml's original features "Check" and "Grade" will be available in this view)

There is currently a JSON import/export feature for exercises, but we plan to work after this PR on enhancing these import/export facilities (e.g. targetting .zip files in the exodir format and/or fetch from external Git exercises repos)

There is now a .zip import/export feature for exercises in the exodir format.
(but we could implement later on another import facility, e.g. to fetch from external Git exercises repos)

Summary of the changes brought by this PR

Add the button Editor in the main view of the web app
(Note: this button is only shown when the token is a teacher one, but the Editor URL itself is not protected (this seems unnecessary) because by design the editor is local-only and doesn't allow inspecting the compiled exercices: it is intended to create new exercices and possibly later fetch from external Git repositories some exercises to be edited)
Change Learnocaml_data to add data-types used by the editor
Add the folder editor containing the corresponding files
(Note: this can be refactored further, especially editor.ml which is strongly inspired by learnocaml_exercise_main.ml)
Do a few changes in toplevel (including adding an optional arg to Learnocaml_toplevel.check) to implement the "Check" and "Generate" features for the "Test" tab, while reusing learn-ocaml's codebase.
Extend the local storage code to save the teacher's exercises that are edited locally.

Overview of the interface

Below is a screenshot [updated] to give a sketch of the corresponding interface:

FYI a statically deployed version (using Github pages) of the previous version of learn-ocaml-editor (0.4.0) is available here: https://pfitaxel.github.io/pfitaxel-demo/

(we'll be able to deploy the latest version of "learn-ocaml+editor" − corresponding to this PR − statically as well if #288 is addressed)

Things to be done before merging this PR

* Rebased on 8ba4e3a Co-authored-by: Manuel Cabarcos-Baulina <crac.master@hotmail.com> Co-authored-by: Romain Grimal <romain.grimal@gmail.com> Co-authored-by: Damien Guagno <guagnodamien@gmail.com> Co-authored-by: Alexandre Perge <alexandre.perge@gmail.com> Co-authored-by: Sophie Rumin <sophierumin@hotmail.fr>

Co-authored-by: Manuel Cabarcos-Baulina <crac.master@hotmail.com> Co-authored-by: Romain Grimal <romain.grimal@gmail.com> Co-authored-by: Alexandre Perge <alexandre.perge@gmail.com> Co-authored-by: Sophie Rumin <sophierumin@hotmail.fr>

* Switch to opam 2 * Compile (_: test_qst_untyped) directly to Test_lib * Disable compilation of TestAgainstSpec for now * Replace src/grader/test_lib.{ml,mli} with ocaml-sf/learn-ocaml@46ded0b * Start improving the polymorph_detector Co-authored-by: Manuel Cabarcos-Baulina <crac.master@hotmail.com> Co-authored-by: Romain Grimal <romain.grimal@gmail.com>

* Refactor strings using {| ... |} * Fix order bug if generating/compiling a testhaut with 10+ questions + Refactor src/state, src/editor to replace most StringMap with IntMap + Remove fetch_test_index that seems unnecessary * Don't ask confirm for individual delete of testhaut question * Reimplementation of the Generate feature using Js Regexp, not (char list)s * Put the code quality section at the beginning * Replace "Dummy_Params" with a shorter identifier "Mock" * Fix Check_Test.ml to take {Prelude,Prepare} contents into account * Improve Check_Test.ml: Auto-select {Prelude,Prepare} tab (and line) if error

Fix the conflicts of the migration, leading to the following changes: * Remove the "testhaut" view to have a stronger focus on the underlying OCaml code for "test.ml"; * New implementation of "Generate" that bypasses the "testhaut" view. The application smoothly compiles and runs, but the following tasks remain: * The "Experiment" feature of the editor is temporarily commented-out; * There is a remaining isssue with the "Check" feature of the "Test" tab; * Internationalization is still to be done.

* Adapt learnocaml_toplevel_worker_main.ml to conditionally load Embedded_grading_cmis.

* If error is at line No. < 0, switch first from "Test" to "Prepare".

…xercise while grading

…on functions

…r when loaded

…t code

* Rely on Editor_lib.extract_functions

…into learn-ocaml-editor

if (one creates a new exercise in the editor and) adds a solution+test then click on Check and very quickly, click on Save&Grade! Now, the "Check" button disables "Save&Grade!" and conversely.

* Fix static deployment of learn-ocaml-editor

Follow-up of 1d2052d

* otherwise the interface is quite unpractical, e.g., using ext_alert with custom buttons would need to copy in the client code, the definition of box_button; so, DRY!

* Add some (commented-out) debug code facilities; * Add some TODO/FIXME/XXX remarks; * Workaround the issue by ignoring the 'exercise.descr' string in editor_lib.ml (otherwise its encoding-then-*decoding* fails)

* origin/master: (23 commits)

* upstream/master: (37 commits) Conflicts: learn-ocaml.opam

It is enabled by default: $ learn-ocaml build <=> $ learn-ocaml build --enable-editor unless we run the command line: $ learn-ocaml build --disable-editor

* In sub-directories: - static/editor/ (*.html) - _opam/share/learn-ocaml/editor/ (*.js|*.html) * Rename one resource as well (s/new_exercise/new-exercise/) * Ensure `learn-ocaml build --disable-editor` does not copy files into www.

To be more precise (using Bash syntax for more clarity): * Beforehand, we used "q_${name}" and "q_${name}_1", "q_${name}_2" which could be problematic if some ${name} ends in "_1". * Henceforth, we use "q_${name}" and "q1_${name}", "q2_${name}".

(albeit learn-ocaml-editor might be internationalized at some point)

Regarding optional e-mails, for the sake of consistency, use the same as that of PR ocaml-sf#362 (branch oauth-moodle-dev). & Do some minor HTML cleanup as well.

(so that 4 is at the top)

yurug · 2022-01-03T06:33:14Z

Hello @erikmd : what's the status of this PR? Should we close it so that you can integrate its feature in a more incremental way?

erikmd · 2022-01-07T02:46:38Z

Hi @yurug, thanks anew, so the feature of this branch is almost ready but there are three things that remain to do:

1. fix the blocking bug I had isolated in this commit: a06f37e
  (I can say I'm up for any help on this issue anytime soon 😅 Cc @yurug @AltGr)
1. finish the refactoring/polishing of (ocaml) code I undertook, fixing along the way a few other minor bugs I have spotted
1. squash the commits in a better release-please compliant way (but we may probably want to keep the commits in an archived branch, for historical purpose)

but no need to close the PR I guess, I'll be able to force-push once the branch has been archived.

Edit: I added these three steps at the end of the description of the issue

erikmd · 2022-01-07T02:57:16Z

To be a bit more specific of the blocking issue partly documented in commit a06f37e → the fact is that in (Learn-OCaml)Editor, tested with Firefox, when we click on Experiment to switch from teacher view to student view (and load the exercise data from the Editor tabs), if the descr.md text is not empty but has e.g. ≥14000 chars, then we get a JS Stack_overflow :-/

Furthermore, this issue was not "present" at all in the initial version of (Learn-OCaml)Editor… (I had done a bisection and noticed the issue was introduced when the src/repo/learnocaml_exercise.ml datatypes had been extended in learn-ocaml master… but I don't remember the exact PR number I had found, sorry for that)

erikmd and others added 17 commits June 24, 2019 01:19

feat: learn-ocaml-editor version 0.2.0

58f6eb2

Co-authored-by: Manuel Cabarcos-Baulina <crac.master@hotmail.com> Co-authored-by: Romain Grimal <romain.grimal@gmail.com> Co-authored-by: Alexandre Perge <alexandre.perge@gmail.com> Co-authored-by: Sophie Rumin <sophierumin@hotmail.fr>

refactor: Remove unneeded files

d748d65

fix: teacher's Check feature in "Test" tab

f417bfc

* Adapt learnocaml_toplevel_worker_main.ml to conditionally load Embedded_grading_cmis.

fix: teacher's Check feature in "Test" tab

59f0207

* If error is at line No. < 0, switch first from "Test" to "Prepare".

fix : fix metadata fields

a61bc99

refactor : some refactor of editor.ml

1d2052d

feat : add experiment feature also update exo_creator

1430be1

feat : zip export of 1 exercise

9a1140f

feat : add zip import for one exercise , also did some refactor

6809473

feat : Add export all

4139663

fix : remove the saving of editor exercise to the state of a proper e…

bac422c

…xercise while grading

feat(export) : add a folder in the zip when exporting one exercise

d23c987

feat (import all): finish Editor_io

d82eb94

leunam217 force-pushed the learn-ocaml-editor branch from 40cf35d to d82eb94 Compare July 19, 2019 12:11

leunam217 added 4 commits July 19, 2019 18:00

feat : add beforeunload management

15780b5

refactor: refactor the majority of dialog boxes with learn_ocaml_comm…

7d76dde

…on functions

refacor : update copyright messages

c55c234

fix : New exercise display is correct

c082d9d

leunam217 force-pushed the learn-ocaml-editor branch from c68ee1a to c082d9d Compare July 22, 2019 09:51

leunam217 and others added 7 commits July 22, 2019 14:56

fix : fix the problem of getting prompt when trying to exit the edito…

c9840c8

…r when loaded

refactor : remove useless template generation when generating the tes…

5d58ab5

…t code

fix : add more expressive message when is an error in the authors field

3b115d1

feat : add id display in exercise list

ff7e53c

refactor: Simplify the code of genTemplate

9581ace

* Rely on Editor_lib.extract_functions

Merge branch 'learn-ocaml-editor' of github.com:pfitaxel/learn-ocaml …

c65c805

…into learn-ocaml-editor

refactor : simplify the id handling in Editor_io.upload

75bf894

erikmd added 11 commits July 7, 2021 14:06

fix: avoid Invalid_argument "Learnocaml_toplevel.protect_execution"

a595709

if (one creates a new exercise in the editor and) adds a solution+test then click on Check and very quickly, click on Save&Grade! Now, the "Check" button disables "Save&Grade!" and conversely.

fix(static/*.html): Add missing leading slash or baseUrl

ec7f365

* Fix static deployment of learn-ocaml-editor

fix(learnocaml_exercise_main.ml): Add missing baseUrl prefix

1693e95

fix(editor.ml): Loading of "/js/learnocaml-toplevel-worker.js"

283ae61

fix(editor_lib.ml): Add missing api_server prefix

53dff98

refactor(learnocaml_exercise_main.ml): Remove duplicate code

991430b

Follow-up of 1d2052d

fix(learnocaml_editor_tab.ml): Add missing api_server prefix

f4ffac8

docs: Add todo-refactor

ab0f4f0

fix(learnocaml_common.mli): Expose (box_button, close_button)

44cd018

* otherwise the interface is quite unpractical, e.g., using ext_alert with custom buttons would need to copy in the client code, the definition of box_button; so, DRY!

fix: Remove unused 'rec'

af85f61

fix: Isolate & Workaround the Stack_overflow raised by 'Experiment'

a06f37e

* Add some (commented-out) debug code facilities; * Add some TODO/FIXME/XXX remarks; * Workaround the issue by ignoring the 'exercise.descr' string in editor_lib.ml (otherwise its encoding-then-*decoding* fails)

erikmd force-pushed the learn-ocaml-editor branch from 7ad6ba8 to a06f37e Compare July 7, 2021 12:09

Merge remote-tracking branch 'origin/master' into learn-ocaml-editor

7db3731

* origin/master: (23 commits)

erikmd added the kind: feature New user-facing feature. label Jul 23, 2021

erikmd added 4 commits August 12, 2021 18:16

Merge remote-tracking branch 'upstream/master' into learn-ocaml-editor

828efb5

* upstream/master: (37 commits) Conflicts: learn-ocaml.opam

feat: Add property enableEditor

388e338

It is enabled by default: $ learn-ocaml build <=> $ learn-ocaml build --enable-editor unless we run the command line: $ learn-ocaml build --disable-editor

feat: Move editor files apart

4a2601f

* In sub-directories: - static/editor/ (*.html) - _opam/share/learn-ocaml/editor/ (*.js|*.html) * Rename one resource as well (s/new_exercise/new-exercise/) * Ensure `learn-ocaml build --disable-editor` does not copy files into www.

QoL: Add a set_progress command in learn-ocaml-editor/Test/Generate

7689962

erikmd force-pushed the learn-ocaml-editor branch from 1b34ab1 to 7689962 Compare August 26, 2021 21:54

erikmd added 5 commits August 27, 2021 23:03

fix: s/Fonction/Function/

507a7d3

(albeit learn-ocaml-editor might be internationalized at some point)

QoL: Specify a JS-RegExp pattern in <input type="text" id="authors" …>

95af8d2

Regarding optional e-mails, for the sake of consistency, use the same as that of PR ocaml-sf#362 (branch oauth-moodle-dev). & Do some minor HTML cleanup as well.

QoL: Reverse the order of items in <select id="difficulty" …>

1a047a5

(so that 4 is at the top)

docs: Improve the default text in learn-ocaml-editor/Question

8aa71e6

erikmd mentioned this pull request Sep 15, 2021

meta-issue: learn-ocaml 0.13.0 release plan and milestones suggestions for upcoming releases #418

Closed

15 tasks

erikmd mentioned this pull request Oct 5, 2021

meta-issue: learn-ocaml 0.13.1+ release plan and milestones suggestions for upcoming releases #437

Closed

15 tasks

erikmd mentioned this pull request Jan 7, 2023

meta-issue: learn-ocaml 0.15.0+ release plan and milestones suggestions for upcoming releases #508

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate learn-ocaml-editor #295

Integrate learn-ocaml-editor #295

erikmd commented Jul 5, 2019 •

edited

yurug commented Jan 3, 2022

erikmd commented Jan 7, 2022 •

edited

erikmd commented Jan 7, 2022 •

edited

Integrate learn-ocaml-editor #295

Are you sure you want to change the base?

Integrate learn-ocaml-editor #295

Conversation

erikmd commented Jul 5, 2019 • edited

Brief overview of the design choices

Summary of the changes brought by this PR

Overview of the interface

Things to be done before merging this PR

yurug commented Jan 3, 2022

erikmd commented Jan 7, 2022 • edited

erikmd commented Jan 7, 2022 • edited

erikmd commented Jul 5, 2019 •

edited

erikmd commented Jan 7, 2022 •

edited

erikmd commented Jan 7, 2022 •

edited