Dunification #197

Drup · 2018-04-09T17:21:14Z

I think we can do it this time! :)
It's not completely finished: I need to look at the ppx failing tests, the ocsigen doc generation and sort out CI. But otherwise, it should be on the right track.

cc @rgrinberg (and @aantron ?), I would appreciate review!

rgrinberg · 2018-04-09T17:31:01Z

tools/jbuild

+  (public_name tyxml.tools)
+  (wrapped false)
+  (modules tyxml_name)
+  (libraries (bytes))))


There's no need to ever use the bytes lib with jbuilder. This library is only useful for building on <4.02 which you can't do anyway with jbuilder. On the other hand, it forces you to declare a dependency on base-bytes which depend on findlib. A big negative IMO.

Fair enough!

rgrinberg · 2018-04-09T17:31:51Z

test/jbuild

+; (alias
+;  ((name   runtest)
+;   (deps   (html_fail.result html_fail.expected))
+;   (action (run diff -dEbZBt html_fail.result html_fail.expected))


If you revive this test, take a look at the diff action. It streamlines the definition of such tests.

I know about it, but it doesn't support custom flags. :/

There's --diff-command to set it for your project. But you're right, this should be customizable per action as well. I'd open a ticket regarding this.

rgrinberg · 2018-04-09T17:34:04Z

META.tyxml.template

@@ -0,0 +1,2 @@
+# JBUILDER_GEN
+archive(byte,toploop) += "tyxml_top.cma"


Wouldn't it be better to use requires and tyxml.top as opposed to archive here? It's a bit more general.

The goal of this is to auto-load tyxml.top in the toplevel when tyxml is #required. Do you have another way of doing that?

requires(byte, toploop) += "tyxml.top" should do the same thing but also handle dependencies if your toplevel lib has dependencies. Anyway, I'm not too sure what's best here. None of it is gonna work with $ jbuilder utop anyway :/

Ah, I see. Yes, your version is better. I should probably open a ticket about better support for this.

Ppx_common can't be used in both the ppx and ppx_reflect.

Drup · 2018-04-11T16:18:39Z

@rgrinberg I fixed the toplevel test in a way that is utterly disgusting, but I don't know how to do better. Do you have any propositions ?

@yomimono

Thanks to @yomimono 's autoci.

rgrinberg · 2018-04-11T18:07:23Z

What you can try and do is manually create a driver executable using your local ppx and then see if you can massage the flags to make it output what you want.

I managed to do this in my ppx_getenv2 example project: https://github.com/rgrinberg/ppx_getenv2/blob/master/test/jbuild#L16

But I used ppx_driver. You can probably get it to work with omp.driver as well.

Drup · 2018-04-11T18:09:54Z

@rgrinberg the ppx is not the problem. The issue is that the code that is executed by the toplevel should have visibility over the tyxml library. So, it needs a -I <the path where .cmis are>. I couldn't find a way to do that propery, so the current version just hardcode the path inside _build.

rgrinberg · 2018-04-11T18:13:41Z

Oh, I see that's a bit shitty. However, can't you build a custom toplevel as an executable, include tyxml normally as a library and then use that to execute your code? You can the preprocessing of the code using the manual driver as I showed.

Drup · 2018-04-11T18:15:32Z

Building a custom toplevel that -linkall against a library only handles the runtime part (cmo and cmas). It doesn't bundle the cmis for typechecking. For that, you still need -I.

rgrinberg · 2018-04-11T18:19:01Z

Oh yeah, I remember having to do that to make $ jbuilder utop work. Sounds like what you need are some decent rules for making toplevels. We should generalize $ jbuilder utop, I suppose. First make the utop dependency optional, and also allow the user to refer to it as a target.

Drup · 2018-04-12T13:21:18Z

Everything seem to be as good as it can possibly be. Merging!

CHANGES: * Dunify This also removes all the deprecated libraries (`tyxml.syntax`, `tyxml.parser`) and removes the ocamlfind library `tyxml.ppx` in favor of `tyxml-ppx`. (ocsigen/tyxml#197 by Drup, Rudi Grinberg and Anton Bachin) * Add simplistic indentation for the Format-based printer (ocsigen/tyxml#187 by Drup) * Allow the ppx to be used for more exotic tyxml instances, such as reactive elements (ocsigen/tyxml#200 by Drup) * Add `Html.of_seq` and `Svg.of_seq`, which allow to easily import HTML parsed with markup in TyXML (ocsigen/tyxml#221 by Drup) ## Elements and attributes * Add Html.txt and Svg.txt as an alias for `pcdata` (ocsigen/tyxml#222 by Drup) * Add noopener link types (ocsigen/tyxml#198 by Jérôme Vouillon) * Slightly relax dt content type (ocsigen/tyxml#193 by Anton Bachin) * Add touch events (ocsigen/tyxml#211 by Malthe Borch) * Fix handling of figcaption in the PPX (ocsigen/tyxml#219 by Drup)

rgrinberg reviewed Apr 9, 2018

View reviewed changes

Drup force-pushed the dunification branch 2 times, most recently from 3b16d5e to af6f7e5 Compare April 9, 2018 19:36

Drup mentioned this pull request Apr 9, 2018

jbuilder WIP #185

Closed

rgrinberg and others added 15 commits April 9, 2018 21:37

jbuilder WIP

873159b

Remove camlp4 things.

4324b3c

Fix and cleanup opam things.

0fb34e6

Remove the old stuff.

e74f9a1

All the jbuilder things.

d3fa431

Ignore warnings on deprecated printers.

e25d40c

Workaround limitation in jbuilder.

3e766eb

Ppx_common can't be used in both the ppx and ppx_reflect.

Restrict to OCaml 4.03.

36ae0ca

Temporarly disable problematic tests.

2d342bd

Remove the dependency from Ppx_reflect to Ppx_common.

36a7db7

Try to kill off old META files.

fc8b095

Add a target for tools.

b423623

Move all documentation and related files to docs/

c416a22

Remove mention of "bytes".

af6f7e5

Fix documentation generation.

7bcc116

Drup force-pushed the dunification branch 3 times, most recently from f53b6b3 to 378fb09 Compare April 11, 2018 16:03

Drup force-pushed the dunification branch from 24b0f05 to 3e69bb2 Compare April 11, 2018 16:29

Drup added 4 commits April 11, 2018 18:31

Attempt at travification.

daad1ac

Thanks to @yomimono 's autoci.

Also build the examples.

04d75d3

Add back the toplevel test.

3e69bb2

Polish the tests a bit more.

078b352

Drup mentioned this pull request Apr 11, 2018

Real custom toplevels ocaml/dune#698

Closed

Drup merged commit ee8bd04 into master Apr 12, 2018

Drup deleted the dunification branch April 13, 2018 14:44

Drup mentioned this pull request Nov 18, 2018

[new release] tyxml-ppx and tyxml (4.3.0) ocaml/opam-repository#13005

Merged

kit-ty-kate mentioned this pull request Jul 18, 2021

Simplexmlparser ocsigen/ocsimore#14

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dunification #197

Dunification #197

Drup commented Apr 9, 2018

rgrinberg Apr 9, 2018

Drup Apr 9, 2018

rgrinberg Apr 9, 2018

Drup Apr 9, 2018

rgrinberg Apr 9, 2018

rgrinberg Apr 9, 2018

Drup Apr 9, 2018

rgrinberg Apr 9, 2018 •

edited

Loading

Drup Apr 9, 2018

Drup commented Apr 11, 2018

rgrinberg commented Apr 11, 2018

Drup commented Apr 11, 2018 •

edited

Loading

rgrinberg commented Apr 11, 2018

Drup commented Apr 11, 2018

rgrinberg commented Apr 11, 2018

Drup commented Apr 12, 2018

		@@ -0,0 +1,2 @@
		# JBUILDER_GEN
		archive(byte,toploop) += "tyxml_top.cma"

Dunification #197

Dunification #197

Conversation

Drup commented Apr 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgrinberg Apr 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Drup commented Apr 11, 2018

rgrinberg commented Apr 11, 2018

Drup commented Apr 11, 2018 • edited Loading

rgrinberg commented Apr 11, 2018

Drup commented Apr 11, 2018

rgrinberg commented Apr 11, 2018

Drup commented Apr 12, 2018

rgrinberg Apr 9, 2018 •

edited

Loading

Drup commented Apr 11, 2018 •

edited

Loading