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

<#/> displays as "[]" rather than "" #50

Closed
slindley opened this issue Jul 28, 2016 · 6 comments
Closed

<#/> displays as "[]" rather than "" #50

slindley opened this issue Jul 28, 2016 · 6 comments

Comments

@slindley
Copy link
Contributor

As Xml is a type alias for [XmlItem], empty XML values are rendered as the empty list.

@slindley slindley added the bug label Jul 28, 2016
@jamescheney
Copy link
Contributor

I thought I'd try to fix this trivial-looking bug but after a little thought I'm not sure why this is a bug. Since Xml is a type alias for XmlItem, one can also write [] instead of <#/>, i.e.

links> [] == <#/>;
true : Bool

Moreover, why is the proposed solution to display it as the empty string "" (which does not parse to an empty Xml value) instead of "<#/>" (which does)?

The only inconsistency I see about the current situation is that there is no way to write a single XmlItem in Links syntax, because writing something like <a>foo</a> yields a single-element list (rendered the same way), while <#><a>foo</a></#> also yields a single-element list that is rendered as <a>foo</a>.

links> var x = <a>foo</a>;
x = <a>foo</a> : Xml
links> var y = <#><a>foo</a></#>;
y = <a>foo</a> : Xml

I think it would be better for <a>foo</a> on its own to have type XmlItem, and then we could view <#><a>foo</a><b>bar</b>...</#> exactly as syntactic sugar for [<a>foo</a>,<b>bar</b>,...], just as one would expect from the aliasing of Xml as [XmlItem].

It might also make more sense to use the Xml type for individual trees rather than hedges (sequences of trees) since an XML document is a single tree, not a sequence of trees.

@slindley
Copy link
Contributor Author

slindley commented Dec 7, 2016

The program page <#/> generates a web page with content []. It would make more sense to generate an empty string, that being the denotation of </#> as serialised XML.

@slindley
Copy link
Contributor Author

slindley commented Dec 7, 2016

We had a discussion about 10 years ago about whether XML literals should have type XmlItem or type [XmlItem]. Unfortunately, I don't remember the justification for the current design, but if you want to change the design then I suggest you think carefully about the consequences. At that point we were considering using an XDuce-style regular expression type system, which we rapidly abandoned. Given that XML is still isomorphic to a plain old algebraic data type, one option would be to define it simply as a type alias - though that could conceivably become inconvenient if we were to try to capture more aspects of XML.

@jamescheney
Copy link
Contributor

jamescheney commented Dec 7, 2016

There seem to be two or more separate issues here:

  • what value does <#/> have? In the current design, its value is []; it is equal to the empty list, which is a value.
  • how should we print the value of <#/> at the REPL? In the current design, it is printed out the same as the empty list, but sequences of XML wrapped in <#>...</#> are printed as XML strings without either enclosing [] brackets or enclosing <#>...</#> tags, since the latter are not legal XML. Other XML values are printed as XML strings that Links will not re-parse to the same value.
  • how should we print it when rendering it as a page)? currently this is done the same as for REPL output, which is what we want for nonempty values but not for <#/>.

I think it is desirable for the default string representation of a value to be a string that parses back to that value, when this makes sense.

So one fix that does not require changing the existing type structure could be to use the existing Value.string_of_value to print out the value of <#/>, as [] as usual, and use a different function render_xml to "render" an XML value as a well-formed xml string when sending a HTTP response. Decoupling these two ways to render XML would also allow printing XML values as lists of XMLItems more uniformly. I'll give that a try before proposing something more radical.

@jamescheney
Copy link
Contributor

Hmm. So the basic problem is that in webif, we call "string_of_value", which does not call "string_of_xml" directly. Instead, it prints the empty list as [], prints nonempty lists of XML values by just concatenating the results using string_of_item, and prints isolated XML(item) values using string_of_item

Likewise we call string_of_value in the REPL and get the same behavior.

When we have an empty list, we have no way of knowing that it is an empty list of XMLitems because we don't have type information.

So would one solution be to change the calls to Value.string_of_value in webif.ml (for the cases returning text/html) to instead be Value.string_of_xml? This wouldn't work exactly because the values are Links values = Links lists of XML(xmlitem)s, not OCaml lists of xmlitems. But something like this should work, because we know the value should be a list of xmlitems, so we can do the right thing even if it is empty.

In any case, I think in a good design page <#/> would not even be well-formed, since the content of a page should be a single XMLitem, not a list of them (possibly none or several).

@jamescheney
Copy link
Contributor

jamescheney commented Jul 27, 2020

Revisiting this, it seems that the following toy program no longer exhibits the undesired behavior:

fun main(_) { page <#/> }
fun setup() {
  var _ = addRoute("",main);
  servePages()
}
setup()

When run, this yields HTML that renders as an empty page, and on viewing the source it appears that [] is no longer being printed when we emit the page. I'm not sure of the reason for this, but I believe the original problem this issue flags is no longer present.

It is still the case that in the REPL the value of the expression <#/> is printed as []. I don't see an easy way to fix this short of adding some kind of run-time tag to Nil to indicate when it was an empty Xml that should be printed as <#/>; if we had something like type classes and defined Xml as a new type rather than as a synonym for [XmlItem] then we could customize the printing behavior to print <#/> but this would entail lots of other changes. In any case, the issue seems to have been about the serialization of empty pages which now seems to be behaving as intended anyway.

So I think this can be closed.

XML, List and pattern matching cleanup automation moved this from To Do to Done Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants