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

Footnote definition does not end #20

Open
critiqjo opened this Issue Apr 10, 2016 · 14 comments

Comments

Projects
None yet
6 participants
@critiqjo
Copy link
Contributor

critiqjo commented Apr 10, 2016

extern crate pulldown_cmark as cmark;

use cmark::{Options, Parser};

fn main() {
    for e in Parser::new_ext("Hello, [^goo]\n\n\
                              [^goo]: world.\n\n\
                              Follow [link][ref]",
                             Options::all()) {
        println!("{:?}", e);
    }
}

The above code outputs (footnote definition extending until the end):

Start(Paragraph)
Text("Hello, ")
FootnoteReference("goo")
End(Paragraph)
Start(FootnoteDefinition("goo"))
Start(Paragraph)
Text("world.")
End(Paragraph)
Start(Paragraph)
Text("Follow ")
Text("[link]")
Text("[ref]")
End(Paragraph)
End(FootnoteDefinition("goo"))
@raphlinus

This comment has been minimized.

Copy link
Owner

raphlinus commented Apr 10, 2016

I sent a PR to take a look at. One question I have is, how should the following be handled:

a: [^b]

[^b]: * a
* b

* c

d

Here we have a conflict between two rules: the footnote should be closed by one blank line, but the list (which is inside the footnote) is closed by two. With the code in the PR, the list [a, b] is inside the footnote def, while c gets its own list. This seems reasonable but I want to think about whether it's truly the best behavior.

Alternatively (since the spec doesn't nail this down), we can dictate that footnotes and lists are both consistently ended by two blank lines.

@notriddle

This comment has been minimized.

Copy link
Contributor

notriddle commented Apr 10, 2016

I (personally) don't see this as a bug. If you look in specs/footnote.txt, multi-paragraph footnotes are deliberately supported, and footnotes are already ended on two blank lines.

@critiqjo

This comment has been minimized.

Copy link
Contributor Author

critiqjo commented Apr 10, 2016

multi-paragraph footnotes are deliberately supported

I see... I did not "expect" that (or lists) in footnotes. Generally, footnotes are a single paragraph of text.

since the spec doesn't nail this down

If we are going to take the route of making footnotes end with a single blank line (which is probably what most people might expect, and markdown is about being intuitive, right?), then deny lists inside it (for consistency in number of blank lines). I'm against it because it will look kinda ugly if footnote definitions (containing lists) are rendered as a numeric list (as in wikipedia).

@critiqjo

This comment has been minimized.

Copy link
Contributor Author

critiqjo commented Apr 11, 2016

Spec for multi-block footnote-definitions.

  1. Footnote definitions are container blocks which cannot be defined within another container.

    > reference[^b].
    >
    > [^b]: but this is not a valid footnote definition
    
  2. A footnote definition can be indented with at most 3 spaces.

       [^b]: footnote text
    
  3. If a sequence of blocks should be contained within a footnote definition, indent it to align with the first non-whitespace character in the definition body.

    [^b]: * list item1 inside footnote
          * list item2 inside footnote
    * list item outside footnote
    
  4. The footnote definition continues as long as the indentation matches (or exceeds):

    [^b]: * list item1 inside footnote
    
    
    
          * list item2 inside footnote
    
    
    
    
          still inside footnote
    
  5. For convenience, the definition body can start in the next line too, with a non-zero indentation (at least N+2 spaces, 0≤N≤3 is the initial indentation):

     [^long-footnote-name]:
       inside footnote
       * list item inside footnote
    
      outside footnote
    

    The definition body cannot start in the next line without the minimum indentation. The following should be treated as an empty footnote definition:

    [^long-empty-footnote]:
     outside footnote
    
  6. Laziness with paragraph continuation:

    [^b]: footnote;
    still in the first line.
          * list item inside footnote;
    within the list item line
    

UPDATE (2017-03-30): Simplify
UPDATE (2017-03-31): Better consistency

@chriskrycho

This comment has been minimized.

Copy link

chriskrycho commented May 2, 2016

FWIW, the reason the spec acts the way it does is because, whatever is most common in writing contexts, multi-paragraph footnotes are a genuine need. (I have from time to time used them myself.) Just as importantly in many ways, pretty much all existing implementations with footnote support treat footnotes as a(n indented) block type, which allows them to have multiple paragraphs within them. So whatever is most purely "intuitive" may not be what's best in this case.

@ScottAbbey

This comment has been minimized.

Copy link
Collaborator

ScottAbbey commented Mar 30, 2017

Regarding the list within footnote issue above:

but the list (which is inside the footnote) is closed by two.

This appears to have changed between CommonMark 0.26 and 0.27.

0.26:

list items may be separated by single blank lines, but two blank lines end all containing lists

0.27:

list items may be separated by any number of blank lines

So it is clear that the c list item must in all cases be a part of the list that contains the b list item.

@ScottAbbey

This comment has been minimized.

Copy link
Collaborator

ScottAbbey commented Mar 30, 2017

Follow-up from #21 (comment):

It looks like this footnote spec is homegrown rather than part of the CommonMark spec. How open is it for change?

I would not have expected list item b to be part of the [^b] footnote definition at all.

It appears as though a footnote definition should treated as a container block -- all indications in this thread are that footnotes are expected to support multiple paragraphs, lists, and other blocks.

If a footnote definition is a container block, it should act very much like list items do in CommonMark. Continuing lines in a footnote definition should be indented to the same level as the first non-whitespace character after the [^b]: item. The rules to end a list item should apply to a footnote definition.

Is there an argument against treating footnotes as an additional type of container block similar to a list item?

@critiqjo

This comment has been minimized.

Copy link
Contributor Author

critiqjo commented Mar 30, 2017

The following text:

foot[^note1], foot[^note2]

[^note1]: text1
[^note2]: text2

Generates:

[
    Start(Paragraph),
    Text("foot"),
    FootnoteReference("note1"),
    Text(", foot"),
    FootnoteReference("note2"),
    End(Paragraph),
    Start(FootnoteDefinition("note1")),
    Start(Paragraph),
    Text("text1"),
    SoftBreak,
    FootnoteReference("note2"),           # note this
    Text(": text2"),
    End(Paragraph),
    End(FootnoteDefinition("note1"))
]

Instead of:

[
    Start(Paragraph),
    Text("foot"),
    FootnoteReference("note1"),
    Text(", foot"),
    FootnoteReference("note2"),
    End(Paragraph),
    Start(FootnoteDefinition("note1")),
    Start(Paragraph),
    Text("text1"),
    End(Paragraph),
    End(FootnoteDefinition("note1"))
    Start(FootnoteDefinition("note2")),
    Start(Paragraph),
    Text("text2"),
    End(Paragraph),
    End(FootnoteDefinition("note2"))
]

Which can be generated if there is a blank line between footnote definitions. Is this intentional too?

@ScottAbbey

This comment has been minimized.

Copy link
Collaborator

ScottAbbey commented Mar 30, 2017

Which can be generated if there is a blank line between footnote definitions. Is this intentional too?

It appears to be intentional as that is covered specifically by a test case in the footnotes.txt spec. I would have expected your example to be two separate footnote definitions if I hadn't read that spec first.

Was this footnote spec copied from somewhere or just made for this program?

@notriddle

This comment has been minimized.

Copy link
Contributor

notriddle commented Mar 30, 2017

I just made it up.

@critiqjo

This comment has been minimized.

Copy link
Contributor Author

critiqjo commented Mar 30, 2017

After going through the spec for lists once again, and keeping the goal to maximize intuitiveness, I rewrote the spec I proposed earlier (in-place). Please take a look. I tried to make it consistent with the current style of specs.

@ScottAbbey

This comment has been minimized.

Copy link
Collaborator

ScottAbbey commented Mar 30, 2017

This example:

[^long-footnote-name]:
  inside footnote
  * list item inside footnote
outside footnote

With the footnote identifier change to a list item marker would be all one list item due to lazy continuation rules. The "outside footnote" text would be part of the "list item inside footnote" (see here)

Considering your second example as a list item rather than a footnote, it seems fine. However, this version should probably all be one footnote due to laziness:

[^b]: text
inside footnote

This version with the subsequent line indented to one space beyond the footnote marker should probably be just like a list item starting with a blank line:

[^b]:
      inside footnote

I agree with your third example.

It seems like the one place this still differs from list items is dealing with a long footnote name. Subsequent blocks inside a list item (except in the case of laziness) need to be indented as far as the first non-whitespace character after the list marker. If the footnote name is very long, this could be impractical.

I think the discussion starting with "Would it help to adopt a two-space rule?" under 5.2.1 Motivation in the CommonMark spec may be relevant here.

Perhaps a two-space rule can be justified by something like "The [^ is the footnote definition indicator, so everything should be indented past that." Not sure. Seems a little messy still.

@critiqjo

This comment has been minimized.

Copy link
Contributor Author

critiqjo commented Mar 31, 2017

@ScottAbbey You're right, that's indeed an inconsistency. I'll update the proposal soon.

UPDATE: Updated

@oberien

This comment has been minimized.

Copy link
Contributor

oberien commented Aug 4, 2018

What's the status on this? I just ran into this problem with the following reference-list:

[^foo]: foo
[bar]: bar

This generates the following events, which make the parser not being able to find the link definition and instead calls the broken_link_callback:

Start(FootnoteDefinition("foo")),
Start(Paragraph),
Text("foo"),
SoftBreak,
Text("[bar]: bar"),
End(Paragraph),
End(FootnoteDefinition("foo"))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.