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

@todo in unordered list breaks html #223

Closed
p-vitt opened this issue Feb 28, 2018 · 4 comments · Fixed by #540
Closed

@todo in unordered list breaks html #223

p-vitt opened this issue Feb 28, 2018 · 4 comments · Fixed by #540
Labels

Comments

@p-vitt
Copy link

p-vitt commented Feb 28, 2018

When you add a @todo to an unordered list, the resulting html gets corrupted.

Small working example:

$ mkdir source
$ cat << EOF > source/test.f90
> program TEST
> implicit none
> write(*,*) 'Hello world!'
> end program
> EOF
$ mkdir test_pages
$ cat << EOF > test_pages/index.md
> title: Documentation
> 
> ## Usage
> - Step 1
> - Step 2
>   @todo this is a todo.
> - Step 3
> 
> An introduction to the usage of the features of whatever.
> You are guided through all required steps from generating stuff,
> configuring the solver and post-processing the results.
> 
> An introduction to the usage of the features of whatever.
> You are guided through all required steps from generating stuff,
> configuring the solver and post-processing the results.
> 
> An introduction to the usage of the features of whatever.
> You are guided through all required steps from generating stuff,
> configuring the solver and post-processing the results.
> EOF
$ cat << EOF > test.md
> project: Test
> src_dir: source
> output_dir: docu
> page_dir: test_pages
> title: Testpage
> md_extensions: markdown.extensions.toc
> 
> Testpage
> ========
> 
> This is a testpage.
> 
> [Here](page/index.html) is the Documentation.
> EOF
$ ford test.md

When you remove the @todo from test_pages/index.md, the file is fine. The diff in the output looks like:

$ diff -u broken.html fine.html
--- broken.html	2018-02-28 15:08:51.000000000 +0100
+++ fine.html	2018-02-28 15:09:17.000000000 +0100
@@ -119,13 +119,12 @@
       <h2 id="usage">Usage</h2>
 <ul>
 <li>Step 1</li>
-<li>Step 2
-</p><div class="alert alert-success" role="alert"><h4>Todo</h4><p>write page</li>
+<li>Step 2</li>
 <li>Step 3</li>
 </ul>
 <p>An introduction to the usage of the features of Musubi.
 You are guided through all required steps from generating meshes,
-configuring the solver and post-processing the results.</p></div>
+configuring the solver and post-processing the results.</p>
 <p>An introduction to the usage of the features of Musubi.
 You are guided through all required steps from generating meshes,
 configuring the solver and post-processing the results.</p>

Is this a bug? Or is it generally not allowed to use @todo in an unordered list?

@cmacmackin
Copy link
Contributor

cmacmackin commented Feb 28, 2018 via email

@ZedThree
Copy link
Member

I think the cleanest approach is to base it on the existing admonition extension.

This is probably a breaking change though, as I think it would involve dropping support for the @endtodo delimiter. I have a proof-of-concept that works with "inline messages" and indented blocks, but not for @todo in lists:

@warning This is a single, inline note

@todo
    This is a larger note with both multiple lines and multiple
    paragraphs, that goes on for a while
    
    This is a big block with paragraphs
    
    ```
    it even includes code blocks
    ```

image

image

@mbraakhekke
Copy link
Contributor

mbraakhekke commented Mar 25, 2022

Replying to #357 (comment)

One idea I had was to use a (markdown) preprocessor to indent the text between the @note and @endnote, and remove the @endnote. This initial pass would then get the text into the correct state for the main pass to work.

Great idea! Perhaps we could take it even further and convert it to the original markdown syntax (!!! note). Then only a preprocessor needs to be added and we can keep the stock admonition extension.

I think the difficulty was because the @endnote is optional, I couldn't work out when the note block was supposed to end. The current implementation looks for either a closing paragraph tag </p> or @endnote.

From the Wiki:

If no such @endnote tag can be found then the note's contents will include until the end of the paragraph where the environment was activated.

I haven't tested it but I guess multiple paragraphs in a note are only possible if @endnote is used. I think it should be possible to implement that in a preprocessor.

My suspicion is that it's just not possible to support the current syntax with an MD extension, but conversely, there are other situations that we would be able to support. I think the balance is in favour of the admonition syntax though.

Right now, I don't see why it wouldn't be possible, but you obviously gave it more thought that I did so far.
Can you give some examples of other situations that we could support?

@mbraakhekke
Copy link
Contributor

See #410 for a first attempt.

I kept the original FORD syntax, (not the one @ZedThree proposed). As far as I can tell everything works fine. (And #223 is fixed)
Contrary the standard MD admonitions, custom titles are not supported since it's hard to combine that with one-line admonitions. We might think of a way to add that, but that's a separate issue.

I implemented an MD preprocessor that converts the FORD syntax for admonitions to the syntax of the built-in MD admonitions extension However, this was not sufficient because the built-in extension uses different CSS classes (admonition as opposed to alert ). So I added a modified version of the MD block processor for admonitions as well.
If we change the FORD CSS style sheets then the modified block processor can be omitted--the standard admonition extension should work, as long as the preprocessor is run first. This would be the best solution, in my view. @ZedThree, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants