Checkboxes and TOC aren't correctly rendered
primercuervo opened this issue · 11 comments
Hi,
This might be more a request for clarification, as it might as well be that I'm just using this gem wrong:
I just stumbled upon this after tweaking with vimwiki for a couple of weeks. I was using Gollum until now with fairly good results, but there's no support for table of contents there, which is a bummer.
I'm uncertain if there's such support here, but I do see in the changelog that there has been work done in that direction. However, with a vanilla configuration just as the one stated in the README.md I see my markdown links for the TOC as plain text. Is this expected?
Also, as the syntax used for the markdown is the GitHub enriched, I was also expecting the checkboxes to be rendered, but that is not the case. Is it also expected or am I missing a configuration step?
Thanks!
No, I don't think you're missing anything, it's just incomplete. The table of contents isn't something I've implemented, and I'm not actually sure how that works at all. On my own wiki I just have an index page and link off from that, TOC sounds useful though.
Gollum is interesting, I'd not heard of it before. I suspect that handles things a little differently from the regular github markdown conversion we're using, but, I'm not sure.
Short answer though: No, the gem doesn't currently do any of this. I think we'd have to be careful how to integrate checkbox fanciness, might need to be a config option so as not to break existing implementations (or bump the gems version I guess, hurrah for being forever pre 1.0 ;). I'm unliklely to make time in the near-term to look at this, but, PRs are always welcome if you feel like having a go!
just playing
- what happens with a regular comment outside of a gollum wiki
Interesting, that the above list gets translated into
<ul class="contains-task-list">
<li class="task-list-item enabled"><input type="checkbox" id="" class="task-list-item-checkbox" checked=""> what happens with a regular comment outside of a gollum wiki</li>
</ul>we probably could/should be doing that.
I'm noticing that the html generated by vimwiki for the default syntax of checkboxes goes well along with the style.css lines as follows:
https://github.com/vimwiki/vimwiki/blob/master/autoload/vimwiki/style.css#L142-L172
I can confirm by inspecting the body that the list comes with ul li.doneX. However, with the vimwiki_markdown generated, they are only generated as lists ul li, and therefore they don't get rendered.
Example of Vimwiki2HTML for wiki sintax, without this gem:
Same portion using this gem and the corresponding suggested config:
I haven't dug further, as this is only a suspicion resultant from trial and error. Getting late here in CEST, might look a bit more on this tomorrow, unless you see an obvious fact that I'm overseeing. This seems like the HTML style isn't being propagated as it should
Yeah, it has been a while since I looked at it, but,
vimwiki_markdown/lib/vimwiki_markdown/wiki_body.rb
Lines 25 to 26 in b34a9b7
I assume there's a pipeline which can be used to give the same list classes, it's just not being done. That may be a quick and easy fix. So, it's not that the HTML style isn't being propagated so much as it's not even attempting to translate it into the github flavoured html.
Adding the tasklist support as described in your comment above was trivial, as expected. I issued a PR for that.
Unfortunately, this tells me that this was not exactly what I was expecting 😅 (you'd notice that I'm still getting to know what vimwiki, in general, does, and all the bells and whistles around it).
The internal HTML translator from vimwiki has a rather neat system for different completion states, changing the HTML tag accordingly:
https://github.com/vimwiki/vimwiki/blob/619f04f89861c58e5a6415a4f83847752928252d/autoload/vimwiki/html.vim#L921-L937
This tag can then be customized with an additional style sheet using this 5-states (or 6 if you count rejected). The tasklist from commonmarker, however, is boolean only for checked and unchecked.
My first thought was to create another extension in a commonmarker fork that implements the vanilla vimwiki functionality, and then call the forked gem in vimwiki_markdwon, but then I wondered if it's worth the effort, given that this is not in the spec of commonmarker: regardless of how easy that framework makes the introduction of the extension, It probably won't get merged there.
So... after issuing the PR here, I probably advocate not to use that and have an internal parsing for the <li> tags in vimwiki_markdown that uses the 6-state checkbox from vimwiki. I'll continue having a look at that. In the meantime, please let me know what you think about this train of thoughts.
Fancy! I had no idea about the 6 different states a checkbox can have. I'm definitely a simple boolean person at heart :) Do you find you use the 6-state checkboxes? I mean, I'm totally happy to just deploy the existing quick fix - like I said - I'll actually use it myself quite happily.
Shame really, simply tweaking the commonmarker option was super simple and works really nicely! Nice work finding the option, I've found that code not the most fun to read through.
It'd be interesting to know how many people are using the 6-state checkboxes and whether it was worth trying to implement it. Still, if you're happy to implement it and write some tests for it, I'm happy to merge it in.
Another thought, I do say with the gem that it's rendered using Github Flavoured Markdown... so, maybe it's legit (even preferable) to just use the simple boolean checkbox.
That said, I'm happy to be persuaded. I guess the [X] style will still be rendered correctly anyway, even if you do implement the 6 state option.
In terms of rendering it will do what you are mentioning, yes. Actually, with the PR above we would be strictly using only GitHub Flavoured Markdown.
The 6 states are more an indication of completion over nested task lists, as follows (see https://github.com/vimwiki/vimwiki/blob/master/doc/vimwiki.txt#L1807)
Parent items should change when their child items change. If not, use
|vimwiki_glr|. The symbol between [ ] depends on the percentage of toggled
child items (see also |g:vimwiki_listsyms|): >
[ ] -- 0%
[.] -- 1-33%
[o] -- 34-66%
[O] -- 67-99%
[X] -- 100%
But the user will only either check or uncheck, making it effectively boolean.
The difference is in the generated HTML, however. If someone uses nested tasklists in their vimwiki, the markdown will generate this multi-state in their text editor. However, with the commonmark solution, only the checked and unchecked will be rendered, leaving the others as plain text. For example:
These Markdown lines (notice that ., o and X in parent lines are set automagically by vimwiki when the user checks any child tasks)
# Test for checks
- [ ] This is one line
- [X] This is a checked line
- [ ] This is a parent line 1
- [ ] This is a child line 1
- [ ] This is a child line 2
- [.] This is a parent line 2
- [X] This is a child line 1
- [ ] This is a child line 2
- [ ] This is a child line 3
- [o] Starship
- [X] Cat
- [X] Dog
- [ ] 42
- [X] This is a parent line 2
- [X] This is a child line 1
- [X] This is a child line 2
- [X] This is a child line 3
Results in this rendering with commonmark:
Additionally, although not cool, vimwiki supports tasklists using asterisks or plus as well. The rendering using asterisks is slightly different:
# Test for checks
* [ ] This is one line
* [X] This is a checked line
* [ ] This is a parent line 1
* [ ] This is a child line 1
* [ ] This is a child line 2
* [.] This is a parent line 2
* [X] This is a child line 1
* [ ] This is a child line 2
* [ ] This is a child line 3
* [o] Starship
* [X] Cat
* [X] Dog
* [ ] 42
* [X] This is a parent line 2
* [X] This is a child line 1
* [X] This is a child line 2
* [X] This is a child line 3
This is because commonmark only does
<li>
<p>[.] This is a parent line 2</p>
<ul>
<li>
<input type="checkbox" checked disabled> This is a child line 1</li>
<li>
<input type="checkbox" disabled> This is a child line 2</li>
<li>
<input type="checkbox" disabled> This is a child line 3</li>
</ul>Ideally, and this is what I'm mentioning, the <li> tags are generated more representatively. Vimwiki generated HTML adds a class to it (namely li class done0 to done4), and the user can render it differently with custom style sheets or not.
So there's a bit of collision there, as I'd agree that GitHub flavored markdown shouldn't necessarily support this. But, then, it imposes a limitation on out-of-the-box vimwiki features (which is what I meant in the PR)
I haven't looked at it, but, rather than forking commonmarker or anything like that, does it allow creating your own extensions? Would it be possible to take the existing tasklist extension, extend it to add the classes we need, and then add that either as a dependency to this gem, or, just add it directly into this gem? That seems like a decent way to approach it. If the only difference is going to be adding some semantic classes to the checkboxes, then it seems like a pretty safe approach.
What do you think?
I just hacked together a different approach, which you can see in #29
The idea there is to enrich the list class to imitate the generated from vimwiki for wiki extensions. I took the style from there as well and added it to the default.tpl to demonstrate how this would look like:
This doesn't work along with the commonmarker approach, because the <li> description is different for both. I believe that we could parameterize these two PRs into one single approach that provides both flavors, but the PRs serve as proof of concept.
Please let me know what you think about this. Going the #29 way would be more similar to what vimwiki provides, which isn't GitHub flavored per se.
BTW I just got the CI feedback which I'll happily deal with after your feedback as well. This is my first time writing in ruby, so please apologize for the idioms and mistakes 😅
I was having some issues with the Bundler gem, and ended up installing a newer version (2.1.4) and modifying the gemfile locally. I'm not sure how this should be dealt with, or even if this is an indication that the dependencies should be bumped.