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

Include VG in Toplevel and exercices #352

Merged
merged 3 commits into from
Jul 7, 2021
Merged

Include VG in Toplevel and exercices #352

merged 3 commits into from
Jul 7, 2021

Conversation

maiste
Copy link
Contributor

@maiste maiste commented Jun 2, 2020

Hi,
this is a PR to partially fix #237.
Content of the PR:

  • It allows using Vg (link) in the toplevel.
  • It displays Vg.image in the toplevel` with pretty_printing.
  • It imports a new function to print svg string
 val print_svg: string -> unit
  • It allows using libraries such as VG in the exercise section.

@yurug yurug marked this pull request as draft June 3, 2020 11:36
@maiste maiste changed the title Include VG in Toplevel Include VG in Toplevel and exercices Jun 5, 2020
@maiste maiste marked this pull request as ready for review July 27, 2020 09:59
@maiste maiste mentioned this pull request Jul 27, 2020
@maiste maiste requested a review from yurug July 31, 2020 12:56
@maiste
Copy link
Contributor Author

maiste commented Jun 28, 2021

Hi,
I've rebased and refactor a bit this old PR. It allows displaying vg image in the toplevel. It needs to be test on another machine: it works on mine.

To be clear, this does not support grading image, only displaying them.

@yurug what's your point of view? Should we merge only the displaying and create another PR for the grading part?

@EmileRolley

This comment has been minimized.

@maiste
Copy link
Contributor Author

maiste commented Jun 29, 2021

However, when I try to open an exercise I've got this error:

It's strange, I don't have this issue. It works on the PopOs version of:

  • Chromium 83.0.4103.116
  • Firefox 89.0.1

@hferee
Copy link

hferee commented Jun 29, 2021

I just tried it as well. The image is properly displayed in the toplevel and the exercise still works.
Thanks for the pull request by the way.

@maiste
Copy link
Contributor Author

maiste commented Jun 29, 2021

OK, @EmileRolley I don't know where it can come from...

@hferee thanks! It's an old PR I've opened, but now it has been refactored, it might be mature for merging (as long as we ensure it works).

@yurug
Copy link
Collaborator

yurug commented Jun 29, 2021

@maiste I have made minor comments.

Co-authored-by: Yann Régis Gianas <yann@regis-gianas.org>
@EmileRolley
Copy link
Collaborator

OK, @EmileRolley I don't know where it can come from...

Okey, after some opam install it finally works!

@yurug
Copy link
Collaborator

yurug commented Jun 30, 2021

@EmileRolley @maiste Anything more? Can I merge?

@EmileRolley
Copy link
Collaborator

@EmileRolley @maiste Anything more? Can I merge?

It's ok for me.

@maiste
Copy link
Contributor Author

maiste commented Jun 30, 2021

The CI seems to stuck on Mac, whereas it wasn't the case before I apply the path. Is it possible to manually run it again?
Edit: It seems good to me now.

@yurug, good for merging to me.

@maiste maiste requested a review from yurug July 5, 2021 16:04
@yurug yurug merged commit 1add7ba into ocaml-sf:master Jul 7, 2021
@yurug
Copy link
Collaborator

yurug commented Jul 7, 2021

Thanks!

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

Successfully merging this pull request may close these issues.

Execute the students code under a predefined environment
4 participants