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

Lists wrap items in <p> tags #57

Closed
obar opened this issue Jun 11, 2021 · 4 comments
Closed

Lists wrap items in <p> tags #57

obar opened this issue Jun 11, 2021 · 4 comments

Comments

@obar
Copy link
Contributor

obar commented Jun 11, 2021

Perhaps this observation isn't an 'issue' as the current behavior is accounted for in the tests, but the processing of lists is different in go-org than it is in an org export to HTML: it adds <p> tags around list items, where org export does not. Is it possible/does it make sense to catch the cases when the list item should be a paragraph within WriteListItem (I'm not sure what those cases are), and otherwise write the item as a string? Below is a simple example.

Sample input

- this is a list
- another point

org-mode export output

<ul>
<li>this is a list</li>
<li>another point</li>
</ul>

(actually it adds a org-ul class by default)

go-org output

<ul>
<li>
<p>this is a list</p>
</li>
<li>
<p>another point</p>
</li>
</ul>
@niklasfasching
Copy link
Owner

The current behavior was easier to implement and didn't have any drawbacks for my use case. Do you have a use case in mind?

@obar
Copy link
Contributor Author

obar commented Jun 12, 2021

I came across this via Hugo, seeing that I was getting extra spacing in a list and assuming it was a theme CSS issue at first. I found that the appearance is different than a list from a markdown file.

I doubt that the semantic effect matters at all, and the visual effect of the extra tags could be done away with easily enough in CSS. I wonder if go-org might be the place to tackle it though, as most themes won't have a provision for li > p so the typical user have to find the cause of the extra space and adjust for it manually.

I took a look at the parser and see how a Paragraph having a list of children nodes makes sense to hold the leaf list items, as opposed to Text which only has a string. Is that what you meant about it being easier to implement this way? If this were worth changing (still not clear that it is!) would it make sense to add a ListLeaf type (works the same as a Paragraph but prints without tags), or is there another approach that would be better? Perhaps it's inelegant, but one would get the same result with less work by wrapping the call to WriteNodes (within WriteListItem) with a helper function that special-cases handling a Paragraph at the leaf position, as I alluded to above.

I'm interested to hear your thoughts on this, thanks for taking a look at my ramblings :)

@niklasfasching
Copy link
Owner

thx for the context. will take a stab at this sometime. without checking I'd probably go with the hack in html writer list item rendering you suggest.

@obar
Copy link
Contributor Author

obar commented Jun 22, 2021

I tried giving it a shot with that approach (more or less), and it's looking good—I'm manually going through the tests as I update the files to make sure, and if I hit an edge I'll ask here.

obar added a commit to obar/go-org that referenced this issue Jun 22, 2021
This also removes extra newlines for simple list items, see changes to
tests for details.

Closes niklasfasching#57
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 a pull request may close this issue.

2 participants