search

aantron / markup.ml

Error-recovering streaming HTML5 and XML parsers
http://aantron.github.io/markup.ml
MIT License
146 stars 16 forks source link

Eliminate subtyping around type signal #59

Closed aantron closed 3 years ago

aantron commented 3 years ago

Several functions in Markup.ml accept narrower types than signal. For example,

https://github.com/aantron/markup.ml/blob/b4b59ae16e6bd802cd913cee853114ea70cfd079/src/markup.mli#L710

These functions are difficult to modify freely, for example as in #58.

The only function in Markup.ml that produces a stream with a narrower type than signal is content:

https://github.com/aantron/markup.ml/blob/b4b59ae16e6bd802cd913cee853114ea70cfd079/src/markup.mli#L548-L550

If this were just (signal, 's) stream -> (signal, 's) stream, it would force users only to add a wildcard case to any pattern matching they do on the signals. Conversely, the benefit would be a library interface that is shorter, easier to understand, and an implementation that is much easier to edit.

aantron commented 3 years ago

plist-xml is the only publicly-visible project I was able to find that would be affected by this. @alan-j-hu, what do you think about replacing content_signal and most row types in Markup.ml with just signal?

I am considering doing this and re-releasing it as part of a retagged 1.0.0.

Sorry about the churn. I realize you just updated your deps to allow Markup 1.0.0, and now this comes up :P

See https://github.com/aantron/markup.ml/issues/58#issuecomment-714215916 for the current motivation behind doing this. I also had similar issues during the original development of Markup, but I managed to massage all the types in the original release to fit.

alan-j-hu commented 3 years ago

Okay, I just opened a PR on ocaml/opam-repository to downgrade markup back to below 1.0.0.