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

With Pandoc 2.8, attributes are added to the header element instead of its parent when --section-divs is enabled #5965

Closed
yihui opened this issue Dec 6, 2019 · 13 comments

Comments

@yihui
Copy link
Contributor

@yihui yihui commented Dec 6, 2019

I noticed a change in Pandoc 2.8 (which might be from 9f984ff), but I'm not sure it is intended. Before 2.8, attributes of a header are added to its parent div (or section) when --section-divs is enabled, such as the class attribute:

$ pandoc -t html4 --section-divs <<< "# Hello {.foo}" 

<div id="hello" class="section level1 foo">
<h1>Hello</h1>
</div>

With 2.8+, the class attribute is added to h1:

$ pandoc -t html4 --section-divs <<< "# Hello {.foo}" 

<div id="hello" class="section level1">
<h1 class="foo">Hello</h1>
</div>

(Note that the id attribute is still on the div instead of h1, though.)

We have plenty of (R Markdown) applications that depend on the previous behavior, i.e., the class attribute being added to the parent div instead of the header itself. For example, previously we could style a whole section by defining CSS rules for .foo { }. Now these rules are only applied to the header elements. We also have JavaScript applications that rely on this behavior. I wonder if there is a chance to restore the old behavior. Thank you!

@jgm

This comment has been minimized.

Copy link
Owner

@jgm jgm commented Dec 6, 2019

Yes, it was intentional. But maybe not a good idea, if it's going to break lots of things. It's probably not too late to role this back.

@jgm

This comment has been minimized.

Copy link
Owner

@jgm jgm commented Dec 7, 2019

I'm about to push a change which would yield the following:

<div id="hello" class="section level1 foo">
<h1 class="foo">Hello</h1>
</div>

That would be okay?

@jgm jgm closed this in 7f4154a Dec 7, 2019
@yihui

This comment has been minimized.

Copy link
Contributor Author

@yihui yihui commented Dec 8, 2019

That will help a lot to us! I still wish the class is only added to the div and not repeated on h1, because previously we could use the selector .foo in CSS/JavaScript without specifying the HTML element, and now our selectors have to be changed to div.foo (and section.foo for HTML5). By comparison, without the class foo on h1, it is still easy to select the header (.foo > h1). Thank you!

@jgm

This comment has been minimized.

Copy link
Owner

@jgm jgm commented Dec 8, 2019

I'd consider stripping the class and other attributes from the header. However, I think we need some of them to be on the header itself, e.g. the number attribute and the unlisted or unnumbered classes. It seemed simpler to leave them all on, but maybe that's not the best approach?

@yihui

This comment has been minimized.

Copy link
Contributor Author

@yihui yihui commented Dec 8, 2019

How about providing a flag like --section-attributes=section|header|both so users can decide by themselves? To me, I want the attributes to be on the div only, so I'd choose section, but you could make both as the default.

@jgm

This comment has been minimized.

Copy link
Owner

@jgm jgm commented Dec 8, 2019

Putting all attributes on section alone would break certain things (e.g. current behavior of unnumbered or unlisted for LaTeX output).

I would really rather avoid multiplying command line options unless there's a really good reason.

Seems like it's just a slight inconvenience for you to have them on headers too, right?

@yihui

This comment has been minimized.

Copy link
Contributor Author

@yihui yihui commented Dec 9, 2019

Okay. If it is too much trouble for you, please feel free to consider this issue resolved. I guess I can deal with the redundant classes on the headers. Again, thank you very much!

@jgm jgm reopened this Dec 9, 2019
@jgm

This comment has been minimized.

Copy link
Owner

@jgm jgm commented Dec 9, 2019

I'll keep this open a bit longer to think about this issue.

@tarleb

This comment has been minimized.

Copy link
Collaborator

@tarleb tarleb commented Dec 9, 2019

I wonder whether could make sense to handle --section-divs as a "transformation", as that would allow to modify the intermediate document with a filter. Adding/removing classes to headers and divs would be much easier that way.

@jgm

This comment has been minimized.

Copy link
Owner

@jgm jgm commented Dec 9, 2019

Currently the "transformations" get applied before the filters, though.

@tarleb

This comment has been minimized.

Copy link
Collaborator

@tarleb tarleb commented Dec 10, 2019

My thought was that this way, filters could modify the doc after transformation has "hierarchicalized" the AST. On the other hand, if one wants to do that, it might make more sense to do everything in the filter.

E.g., this restores the original behavior (if one replaces --section-divs with --lua-filter=section-divs.lua:

-- file: section-divs.lua
function hierarchicalize (doc)
  return pandoc.Pandoc(
    pandoc.utils.make_sections(true, nil, doc.blocks),
    doc.meta
  )
end

function add_classes (div)
  local header = div.content[1]
  if header and header.t == "Header" then
    div.classes:extend(header.classes)
    div.classes:extend({"level" .. header.attributes.number})
    header.classes = {}
    header.attributes.number = nil
    return div
  end
end

return {
  {Pandoc = hierarchicalize},
  {Div = add_classes}
}
@yihui yihui mentioned this issue Dec 10, 2019
3 of 3 tasks complete
@jgm

This comment has been minimized.

Copy link
Owner

@jgm jgm commented Dec 10, 2019

Ah, I see. It wouldn't be an easy change to make this a transformation. Currently makeSections gets called in the writers (and in some shared functions), and for various reasons (not just the use of --section-divs). Moving all of that logic to App may not make sense, but it may be worth exploring.

Or we could try to restore more or less the old behavior, where everything (except possibly things like unlisted if they need to be on headers) goes to the section Div.

@jgm

This comment has been minimized.

Copy link
Owner

@jgm jgm commented Dec 11, 2019

Let's try this for now, where the attributes are the same on headings and divs.
It seems fairly harmless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.