yuin / goldmark

:trophy: A markdown parser written in Go. Easy to extend, standard(CommonMark) compliant, well structured.
MIT License
3.68k stars 255 forks source link

[RFE] Add class on ul/ol wrapper of task-list? #113

Closed cipherboy closed 4 years ago

cipherboy commented 4 years ago

I'd like to consider adding classes to containers of extensions. Consider a task list:

- [ ] item 1
- [ ] item 2

This'll get rendered into the following HTML (for instance -- I didn't execute goldmark here):

<ul>
  <li><input type="checkbox" checked=""> item 1</li>
  <li><input type="checkbox" checked=""> item 1</li>
</ul>

There's no way to differentiate (in CSS) between a <ul> with a task list underneath and a regular bulleted list. pandoc solves this problem by adding a class="task-list" to the <ul>. GitHub adds the class contains-task-list.

I think we could solve this in three ways:

  1. Directly extend the <ul> rendering classes to detect if it contains a task-list. Solve it for this one case.
  2. Add a more generic mechanism for extensions to inform the AST of modifications to parent elements. Converting e.g., <p> to <div>, adding arbitrary number of classes on arbitrary parent elements, ... -- gives control of modifying the AST to extensions rather than external callers.
  3. Add a configuration parameter that allows the calling application to configure what classes they'd like to have on what classes. Allow introspection of the children elements so the application can modify it as they desire. Probably involves walking the AST and executing a callback function on each step.

Thoughts? I'm happy to implement whatever you decide, I'd just like to leave the design decision to you as it is your project.


Boilerplate below:

Please answer the following before submitting your issue:

  1. What version of goldmark are you using? : v1.1.25.
  2. What version of Go are you using? : v1.13
  3. What operating system and processor architecture are you using? : linux/amd64
  4. What did you do? : Rendered a task list (like the above).
  5. What did you expect to see? : I hoped for a class on the parent ul/ol element, to different it from a regular bulleted/numbered list.
  6. What did you see instead? : a bare ul/ol.
  7. Did you confirm your output is different from CommonMark online demo or other official renderer correspond with an extension?: Yes, GitHub and pandoc give the ul a class, to help with styling.
  8. (Feature request only): Why you can not implement it as an extension?: I'd like to. I need to understand what the scope of this feature should be, before implementing it. I'm filing this issue to begin a discussion.
    • You should avoid saying like "I'm not familiar with this project" "I'm not a Go programmer" as far as possible. This is an open source project and a library for Go programmers. I encourage you to strive to read source codes.
    • I absolutely welcome questions that are difficult even if you read the source codes.
yuin commented 4 years ago

pandoc renders an unordered task list with <ul class="task-list">. GitHub uses the class contains-task-list.

Official specification is here and as you can see task lists do not have any classes. It means pandoc and Github implementation do not stick with official spec. So this should be an extension and not be included in the main repository.

I would recommend you to implement an ASTTransformer.

  1. Walk through AST using ast.Walk
  2. If we find an ast.TaskCheckBox node,
    1. Get an ast.List node by n.Parent().Parent()
    2. Add classes: listNode.SetAttributeString("class", "task-list")
zeripath commented 4 years ago

Thanks @yuin I thought this might be your suggested solution. I've implemented something similar in our transformer. I guess that this issue could now be closed.

Ref: go-gitea/gitea#10798

yuin commented 4 years ago

@zeripath That's good to know, thanks!