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

IAL are ignored on footnotes #194

Closed
cabo opened this issue Dec 12, 2014 · 12 comments
Closed

IAL are ignored on footnotes #194

cabo opened this issue Dec 12, 2014 · 12 comments
Assignees

Comments

@cabo
Copy link

@cabo cabo commented Dec 12, 2014

IALs from footnote references are available to the converter, while IALs from the footnotes themselves aren't. This is not a problem for the HTML converter, which seems to ignore attributes on footnote references anyway (only uses el.value -- is that a bug in its own right?), but it would be nice to be able to get at the attributes in kramdown-rfc2629.

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 4, 2015

Could you please specify a sample document where an IAL is assigned to a footnote and how this should be represented in the HTML converter? I know you want this for kramdown-rfc2629 but it would help me understand the problem better.

When I use this input document:

this is [^5]{:.test}

[^5]: test

the value of el.attr in #convert_footnote in the HTML converter contains {"class"=>"test"}.

So the class is assigned but isn't yet used which should probably be changed.

@cabo
Copy link
Author

@cabo cabo commented Feb 4, 2015

Example:

this is [^5]

[^5]: test
{:.test}

I don't have a string opinion how this should be represented -- I mainly want to get at the data :-)

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 5, 2015

Footnote definitions cannot be assigned IALs as there is no content element to assign the options to. As shown above you can assign an IAL to a footnote marker itself - would that help?

@cabo
Copy link
Author

@cabo cabo commented Feb 5, 2015

Yes, that's what I'm doing now. But I seem to remember the IAL on the footnote (see example) is processed well but discarded only at the place where the footnote is constructed. (The context is https://tools.ietf.org/html/draft-iab-xml2rfcv2-00#section-2.12, where I'd prefer the "source" attribute to be written on the footnote definition and not the footnote marker. Footnotes are supposed to be out of the way; having the source at the place where the footnote is referenced as opposed to where it is defined is distracting and also somewhat nonsensical. And I seem to remember the data is already there, just discarded in parse_footnote_marker at the Element.new, third parameter, which should be like fn_def.attr and not nil. Maybe I should just be monkey-patching that, but that strikes me as less maintainable.)

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 5, 2015

A block IAL after a footnote definition is not discarded but applied to the block afterwards:

[^5]: test
{:.test}
para

is transformed to:

<p class="test">para</p>

This is because a footnote definition is a non-content element for kramdown (like a link definition) and therefore cannot be associated with an IAL.

So even if we would assign fn_def.attr, it would not work because the IAL got not applied to the footnote definition in the first place. The footnote definition itself also isn't in the element tree.

@cabo
Copy link
Author

@cabo cabo commented Feb 5, 2015

Well, I'm not sure "non-content element" means much to me here. My point is that, if there is something that looks like an IAL associated to a footnote definition, that would be a useful way to get the attributes there. (I'm assuming we'll have IALs on, say, link definitions, too.) Much better from a "readable markup" position than to write down the IAL at the point of reference, which is distracting from the content there. RFCXML may have a more plausible application of that than HTML, so that's maybe why this is more obvious to me.

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 5, 2015

You are perfectly right, this is probably not really obvious and was a bad choice.

The only problem I have is that if I change the semantics now (i.e. allow assigning an IAL to footnote or link definitions) it would probably break documents, although, in this case, the chances would be very, very low.

I will think a bit more about this before the next release.

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 28, 2015

@cabo It's been a while and I think I have a solution that would satisfy all needs:

  • If an IAL is after a footnote definition, link definition or abbreviation definition and if a blank line follows, it is assigned to the definition and not discarded.
  • The information from the IAL is applied in the following way:
    • link definition: The attributes from the IAL are added to the attributes of the link.
    • footnote definition: The data will be available but is not used by kramdown itself.
    • abbreviation definition: The attributes from the IAL are added to the attributes of the abbreviation.

This is backwards-compatible since now such an IAL was just discarded. And you would have the information from the IAL available.

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 28, 2015

Actually, I have decided to omit the "and if a blank line follows" part. It is very unlikely that a construct like

[link def]: some.html
{: .some-ial}
some block element

has been used somewhere. I will note that in the news announcement but don't expect any problems.

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 28, 2015

@cabo I have implemented the changes so that block IALs can be applied to link, footnote and abbreviation definitions.

The IALs applied to link and abbreviations definitions are used by built-in kramdown converters but attributes from IALs applied to footnote definitions are currently not. However, they are available in the :footnote elements (ie. IALs converted to attrs in el.attr and the IALs themselves in el.options[:ial], as usual).

@gettalong gettalong closed this Feb 28, 2015
gettalong added a commit that referenced this issue Feb 28, 2015
The applied attributes are currently not used in any converter
shipped with kramdown.

Fixes #194
@cabo
Copy link
Author

@cabo cabo commented Feb 28, 2015

@gettalong
Copy link
Owner

@gettalong gettalong commented Feb 28, 2015

Thanks for testing it out!

And please open a new issue if find the problem with the XML markup!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants