Users can now add custom HTML before the closing head tag and the closing body tag

[tmbb]

I think I've covered all test cases that make sense, but there is still much about ex_doc which I don't know in detail.

[josevalim]

@tmbb instead of changing existing tests, can you please add a new test for this feature? Our ex_doc tests tend to really grown in size with time and that makes it difficult to know what they are really testing. Thank you!

[tmbb]

Done

[josevalim]

❤️ 💚 💙 💛 💜

[milmazz]

@tmbb @josevalim Why? This should also work with EPUB documents, at the end, an EPUB file is just a bunch of .xhtml files.

[tmbb]

Because I've only added this functionality to the HTML templates :) If you want it, I can add it to the EPUB ones.

[josevalim]

I would hold before adding this to ePub because it is not guaranteed you would like the exact same changes in both. Maybe the flag should have html in its name? -- *José Valimwww.plataformatec.com.br <http://www.plataformatec.com.br/>Founder and Director of R&D*

[tmbb]

@josevalim I hadn't thought of that. I can add an html_ prefix or something. It'd become more future-proof. Or we can have the HTML option without the prefix and the EPUB with an epub prefix. I don't know. You should decide.

[josevalim]

maybe before_closing_html_head_tag?

[milmazz]

I would hold before adding this to ePub because it is not guaranteed you would like the exact same changes in both

@josevalim Good point!, but...

Maybe the flag should have html in its name?

I don't think it is necessary and it will pollute the configuration, suppose that in the future we add something like before_closing_epub_head_tag....

IMHO the author is responsible to control this behavior, for example, if the author wants different styles for HTML and EPUB can do the following:

<style type="text/css" media="screen">
...
</style>

<style type="text/css" media="amzn-kf8" [or "not amzn-kindle"]>
...
</style>

The same can be applied with JS.

wdyt?

[tmbb]

An the epub version would be befor_closing_epub_head_tag? Sounds good to me.

[tmbb]

@milmazz Do you trust CSS support of EPUB readers that much? I thought it was pretty bad.

[milmazz]

@tmbb EPUBv3 supports CSS2.1 + CSS3.

More info: http://www.idpf.org/epub/30/spec/epub30-overview.html#sec-rendering

[tmbb]

Hm... I'm still not convinced. Having different keys is simpler, and costs nothing except fro two more keys in the config... I'm against pure CSS solutions for this.

[milmazz]

@tmbb I'm just saying that it's a common practice to create media-specific (screen, projection, print, etc.) styles with CSS and it is even recommended, to give you a few examples alistapart.com, and Eric Meyer

Also, we already provide some styles for the print media in ExDoc with:

@media print {
// ...
}

HTH

[tmbb]

Hm... Interesting... I didn't know CSS was so versatile. There are probably ways to generate PDFs from ExDoc too, right?

[tmbb]

@josevalim Could you weigh in on how this should be done? I'd like to have a new release of ex_doc with this new functionality so that I can use the improved ex_doc to document my own packages (I cant add the github master as a dependency) :)

[OvermindDL1]

Why not make the options like before_closing_head_tag take a function instead and pass in an argument defining 'what' is being generated for?

[tmbb]

@OvermindDL1 Maybe not a function, but a map. That way you can match the keys on the map, and if all matches fail you return the empty string. With a function the burden of having a clause for each option is on the user.

[OvermindDL1]

@OvermindDL1 Maybe not a function, but a map. That way you can match the keys on the map, and if all matches fail you return the empty string. With a function the burden of having a clause for each option is on the user.

You can always catch if there is a match/argument error and return an empty string then, but I personally thing it is better to be explicit than implicit. They may actually want a match error to remind them that they need to add some functionality to another format for example. it is not hard to add a _ -> "" if they truly want to return the empty string for anything they do not support.

[tmbb]

You're right, a function is better than a map, unless we actually raise an error when the map key doesn't match to notify the user.

[josevalim]

@milmazz my concern is that CSS is not the only reason to customize this. What if you want to include some javascript that doesn't really make sense in a book?

[tmbb]

@josevalim I agree that it's better to have separate inclusions for EPUB and HTML. The questions are:

1. Do people want these features in the EPUB format? I believe it might make sense to have them, especially if you want to use some form of syntax highlighting that doesn't depend on Javascript

2. If so, then: How do we handle both formats? Do you prefer a function like @OvermindDL1 suggested? Or just two more config options? I don't think we should be afraid of these config options. After all, their impact on the code is pretty much zero: It's just <%= @options.option_name %> in the right place after all...

[milmazz]

@josevalim Ok, considering your concern I think that the best approach in this scenario is the one suggested by @OvermindDL1, both options (before_closing_{head,body}_tag) can take a function and its first argument is the format, then the author is responsible of the behavior of that function per formatter.

I prefer the approach mentioned before than adding more options in our configuration, suppose that we include support for another format, so then we have something like this:

• before_closing_html_head_tag
• before_closing_epub_head_tag
• before_closing_another-format_head_tag
• ...
• before_closing_html_body_tag
• before_closing_epub_body_tag
• before_closing_another-format_body_tag
• ...

wdyt?

/cc @tmbb @OvermindDL1

[tmbb]

In one option you have:

docs: [
  before_closing_head_tag: &before_closing_head_tag/1
]

def before_closing_head_tag(:html), do:  "..."
def before_closing_head_tag(_), do: ""

def before_closing_body_tag(:html), do:  "..."
def before_closing_body_tag(_), do: ""

In the other you have:

docs = [
  before_closing_html_head_tag: "..."
  before_closing_html_head_tag: "..."
  before_closing_epub_head_tag: "..."
  before_closing_epub_head_tag: "..."
]

This version is simpler and might play well with formats that don't have a head or body tag.

[tmbb]

But it's probably just a matter of preference.

[milmazz]

Sure, seems like a matter of preference, but please consider that when you add a new option to ExDoc you need to change the struct, add the corresponding spec, update the docs for Mix task, update the docs for the command line. Also, from the user point of view, they would need to invest more time to know all the options that ExDoc gives them.

HTH

[josevalim]

Let's go with the function. @tmbb can you please change it accordingly? Thank you!

[tmbb]

Yes, but probably only on monday.

If my new "anti-highlightjs" hack works, could we have a release after support for other formats (in the form of a function) is merged?

I'd like to be able to use my alternative code highlighter to document my own hex packages, and I can obly do it once there's been a release.

[tmbb]

Done #768. My anti Highlightjs hack works. I think we should have a new release.