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

Add all inline elements to avoid wrong space breaking #76

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sorawee
Copy link

@sorawee sorawee commented Nov 8, 2018

Fix #75

@sorawee
Copy link
Author

sorawee commented Nov 8, 2018

Note: the failure in travis-ci is due to nondeterminism of the setup. It's not related to my PR. You can request for a rebuild (I don't have a permission to do so), and it probably will now pass.

@greghendershott
Copy link
Owner

greghendershott commented May 1, 2019

I've been swamped and only recently was able to dig in and think about this.

  • I like the idea of using the updated HTML5 inline elements.

  • I'm unsure about changing the behavior so that there isn't a leading newline, anymore.

Many years ago when I was new to Racket, I found it strange that this writes a leading newline:

(require xml)
(display-xml/content
 (document-element
  (read-xml (open-input-string
             "<html></html>"))))

=>


<html>
</html>

And then in the Racket web-server I saw it wrote a doctype, but not a newline, followed by display-xml. The leading newline from the latter, seemed to be an intentional feature.

So when I did this markdown package for use in frog, I emulated this in display-xexpr.

That was what was nagging at me, about this PR. I knew there was something, but I couldn't find time to focus and recall that.


Fast forward some years, and frog doesn't even use display-xexpr anymore to write the page. It switched to using webserver/templates. So the change would be N/A for Frog.

However I wonder if it would break anything else. e.g. Pollen uses this package.


TL;DR I think the PR is good, except we should keep the leading newline. If that really bugs you, then we should add an optional keyword argument for it, which defaults to true for backward compatibility.

@greghendershott
Copy link
Owner

@sorawee If you're reading this in email, the previous comment is better read here on GitHub -- I made a few edits to clarify. I don't think GitHub re-sends edits as emails.

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.

xexpr->string screws up whitespace
2 participants