ocsigen / tyxml

Build valid HTML and SVG documents
https://ocsigen.org/tyxml/
Other
163 stars 57 forks source link

Emit Unsafe.string_attrib for _-prefixed attributes in syntax ppxs to support non-standard attribute names #296

Closed cemerick closed 2 years ago

cemerick commented 2 years ago

Fixes #295

(also includes more generalized kebab-casing for attributes from jsx)

cemerick commented 2 years ago

FWIW, I've been using this in anger for a week or so, and it's working out quite nicely to emit htmx-flavoured HTML.

Any thoughts on the approach?

Drup commented 2 years ago

Sorry about the late response.

Interesting escape hatch. Why do you need a_unchecked instead of just Unsafe.string_attrib ?

Since re is already in the dependency cone, if your patch gets in, we definitely want to use that instead of str. There is also already some snake-to-caml case conversion tooling somewhere (in /tools maybe ?)

cemerick commented 2 years ago

Interesting escape hatch. Why do you need a_unchecked instead of just Unsafe.string_attrib ?

Oh, maybe I don't. I didn't think to (re)use Unsafe bits, but I guess that's what they're there for.

I'll tweak that up and swap Str -> Re.

cemerick commented 2 years ago

I have updated my fork (and this PR) to:

  1. Eliminate the use of Str in favor of Re (the case conversion stuff referenced /tools isn't sufficient; inputs to that kebab_case function can have a mix of kebab, snake, and camel case, alas)
  2. Remove the a_unchecked and `Unchecked additions, using Unsafe.string_attrib instead. Hopefully my handling of the codegen in that case is reasonable (I didn't see any easy way to reuse any of the existing "parsers" there).
cemerick commented 2 years ago

I've updated to include top-level compilation of the necessary patterns, and added a section to the ppx/jsx docs.

(I haven't been able to "test" the latter though, as I had trouble getting the wikidoc to build; make in the docs directory does produce the API reference, but dies before it generates the narrative wiki content.)

Hopefully I didn't faff up the wiki documentation markup. :crossed_fingers:

Drup commented 2 years ago

Thanks @cemerick