Skip to content

Commit 0996db0

Browse files
NightFlyerAshe Connor
authored andcommitted
Fix bug where tasklist extension was using union in two ways. (commonmark#169)
The tasklist extension was using the "as" union both as a list (in `tasklist.c:parse_node_item_prefix`) and as an opaque (in `tasklist.c:open_tasklist_item`). This meant that strange bugs could occur because the underlying union memory was being overwritten. It manifested when using nested task lists indented by 4 spaces. To fix this, I added the "checked" field to the `cmark_list` structure. This allows the tasklist extension to use the `as.list` union member in all its operations. This is appropriate because a tasklist item is a list item in all essentials -- and even shares the CMARK_NODE_ITEM type.
1 parent b47d804 commit 0996db0

File tree

3 files changed

+66
-17
lines changed

3 files changed

+66
-17
lines changed

extensions/tasklist.c

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,15 @@ int cmark_gfm_extensions_set_tasklist_item_checked(cmark_node *node, bool is_che
2323
if (!node || !node->extension || strcmp(cmark_node_get_type_string(node), TYPE_STRING))
2424
return 0;
2525

26-
if (is_checked) {
27-
node->as.opaque = (void *)CMARK_TASKLIST_CHECKED;
28-
return 1;
29-
}
30-
else {
31-
node->as.opaque = (void *)CMARK_TASKLIST_NOCHECKED;
32-
return 1;
33-
}
26+
node->as.list.checked = is_checked;
27+
return 1;
3428
}
3529

3630
bool cmark_gfm_extensions_get_tasklist_item_checked(cmark_node *node) {
3731
if (!node || !node->extension || strcmp(cmark_node_get_type_string(node), TYPE_STRING))
3832
return false;
3933

40-
if (node->as.opaque == (void *)CMARK_TASKLIST_CHECKED) {
34+
if (node->as.list.checked) {
4135
return true;
4236
}
4337
else {
@@ -95,11 +89,7 @@ static cmark_node *open_tasklist_item(cmark_syntax_extension *self,
9589
cmark_parser_advance_offset(parser, (char *)input, 3, false);
9690

9791
// Either an upper or lower case X means the task is completed.
98-
if (strstr((char*)input, "[x]") || strstr((char*)input, "[X]")) {
99-
parent_container->as.opaque = (void *)CMARK_TASKLIST_CHECKED;
100-
} else {
101-
parent_container->as.opaque = (void *)CMARK_TASKLIST_NOCHECKED;
102-
}
92+
parent_container->as.list.checked = (strstr((char*)input, "[x]") || strstr((char*)input, "[X]"));
10393

10494
return NULL;
10595
}
@@ -110,7 +100,7 @@ static void commonmark_render(cmark_syntax_extension *extension,
110100
bool entering = (ev_type == CMARK_EVENT_ENTER);
111101
if (entering) {
112102
renderer->cr(renderer);
113-
if (node->as.opaque == (void *)CMARK_TASKLIST_CHECKED) {
103+
if (node->as.list.checked) {
114104
renderer->out(renderer, node, "- [x] ", false, LITERAL);
115105
} else {
116106
renderer->out(renderer, node, "- [ ] ", false, LITERAL);
@@ -131,7 +121,7 @@ static void html_render(cmark_syntax_extension *extension,
131121
cmark_strbuf_puts(renderer->html, "<li");
132122
cmark_html_render_sourcepos(node, renderer->html, options);
133123
cmark_strbuf_putc(renderer->html, '>');
134-
if (node->as.opaque == (void *)CMARK_TASKLIST_CHECKED) {
124+
if (node->as.list.checked) {
135125
cmark_strbuf_puts(renderer->html, "<input type=\"checkbox\" checked=\"\" disabled=\"\" /> ");
136126
} else {
137127
cmark_strbuf_puts(renderer->html, "<input type=\"checkbox\" disabled=\"\" /> ");
@@ -143,7 +133,7 @@ static void html_render(cmark_syntax_extension *extension,
143133

144134
static const char *xml_attr(cmark_syntax_extension *extension,
145135
cmark_node *node) {
146-
if (node->as.opaque == (void *)CMARK_TASKLIST_CHECKED) {
136+
if (node->as.list.checked) {
147137
return " completed=\"true\"";
148138
} else {
149139
return " completed=\"false\"";

src/node.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ typedef struct {
2121
cmark_delim_type delimiter;
2222
unsigned char bullet_char;
2323
bool tight;
24+
bool checked; // For task list extension
2425
} cmark_list;
2526

2627
typedef struct {

test/extensions.txt

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,11 +750,59 @@ Autolink and tables.
750750
</ul>
751751
````````````````````````````````
752752

753+
Show that a task list and a regular list get processed the same in
754+
the way that sublists are created. If something works in a list
755+
item, then it should work the same way with a task. The only
756+
difference should be the tasklist marker. So, if we use something
757+
other than a space or x, it won't be recognized as a task item, and
758+
so will be treated as a regular item.
759+
753760
```````````````````````````````` example
754761
- [x] foo
755762
- [ ] bar
756763
- [x] baz
757764
- [ ] bim
765+
766+
Show a regular (non task) list to show that it has the same structure
767+
- [@] foo
768+
- [@] bar
769+
- [@] baz
770+
- [@] bim
771+
.
772+
<ul>
773+
<li><input type="checkbox" checked="" disabled="" /> foo
774+
<ul>
775+
<li><input type="checkbox" disabled="" /> bar</li>
776+
<li><input type="checkbox" checked="" disabled="" /> baz</li>
777+
</ul>
778+
</li>
779+
<li><input type="checkbox" disabled="" /> bim</li>
780+
</ul>
781+
<p>Show a regular (non task) list to show that it has the same structure</p>
782+
<ul>
783+
<li>[@] foo
784+
<ul>
785+
<li>[@] bar</li>
786+
<li>[@] baz</li>
787+
</ul>
788+
</li>
789+
<li>[@] bim</li>
790+
</ul>
791+
````````````````````````````````
792+
Use a larger indent -- a task list and a regular list should produce
793+
the same structure.
794+
795+
```````````````````````````````` example
796+
- [x] foo
797+
- [ ] bar
798+
- [x] baz
799+
- [ ] bim
800+
801+
Show a regular (non task) list to show that it has the same structure
802+
- [@] foo
803+
- [@] bar
804+
- [@] baz
805+
- [@] bim
758806
.
759807
<ul>
760808
<li><input type="checkbox" checked="" disabled="" /> foo
@@ -765,4 +813,14 @@ Autolink and tables.
765813
</li>
766814
<li><input type="checkbox" disabled="" /> bim</li>
767815
</ul>
816+
<p>Show a regular (non task) list to show that it has the same structure</p>
817+
<ul>
818+
<li>[@] foo
819+
<ul>
820+
<li>[@] bar</li>
821+
<li>[@] baz</li>
822+
</ul>
823+
</li>
824+
<li>[@] bim</li>
825+
</ul>
768826
````````````````````````````````

0 commit comments

Comments
 (0)