Allow html in crate docs.

[zzmp]

This makes the in-header, markdown-before-content, and markdown-after-content options available to rustdoc when generating documentation for any crate.

Before, these options were only available when creating documentation from markdown. Now, they are available when generating documentation from source.

This also updates the rustdoc -h output to reflect these changes. It does not update the man rustdoc page, nor does it update the documentation in the rustdoc manual.

[huonw]

FWIW, this isn't allowing markdown, this is allowing HTML to be included inline. Could you update the commit message?

[huonw]

This can just be use markdown::load_external_files;

[zzmp]

How so? I'm using the fn from src/librustdoc/markdown. Don't I need this to differentiate it?

Also, to dive a bit deeper, I wanted to avoid copy-pasting this function when it was available in another module but, in doing so, I've made it publicly available through the rustdoc crate. Is that OK? Should I, instead of using use statements, simply be copying the source of this function (and the macro/function it depends on) to the render.rs file?

I tend to think this should be refactored to avoid exposing it unnecessarilly.

[huonw]

imports are relative to the root of the crate by default (equivalent to /... in a file system) and markdown is declared at the root. The relative import works, but it's not necessary. (This is like being in a directory /html/render and accessing /markdown/load_external_files via ../../markdown/load_external_files but instead /markdown/load_external_files works and is simpler.)

Also, to dive a bit deeper, I wanted to avoid copy-pasting this function when it was available in another module but, in doing so, I've made it publicly available through the rustdoc crate. Is that OK?

Yes, but it would be nice to avoid it.

Should I, instead of using use statements, simply be copying the source of this function (and the macro/function it depends on) to the render.rs file?

No definitely not.

I tend to think this should be refactored to avoid exposing it unnecessarily.

Yep, my longer comment on the PR itself addresses by putting the loading into one place.

[huonw]

Thanks for doing this work!

Unfortunately there's ways in which this can be improved:

• the options can be renamed from --markdown-... (which reflected that they could only be used with standalone markdown files) to --html-... (reflecting that they include raw html)
• now that both forms of documentation generation use these files, the reading should move to be common to both (i.e. in lib.rs). I'm thinking something like this:

In markdown.rs (it should probably move elsewhere, but I'm not sure the best way to manage it... maybe move load_string, load_or_return! and load_external_files to their own external_files module?):

pub struct ExternalHtml {
    pub in_header: String,
    pub before_content: String,
    pub after_content: String
}

impl ExternalHtml {
    pub fn load(in_header: &[String], before_content: &[String],
                after_content: &[String]) -> Option<ExternalHtml> {
        match (load_external_files(in_header),
               load_external_files(before_content),
               load_external_files(after_content)) {
            (Some(ih), Some(bc), Some(ac)) => {
                Some(ExternalHtml {
                    in_header: ih,
                    before_content: bc,
                    after_content: ac
                })
            }
             _ => None
        }
    }
}

...

pub fn render(..., external_html: &ExternalHtml) { ...

In lib.rs

use markdown::ExternalHtml;

// ...

// possibly on on L178
let external_files = match ExternalHtml::load(matches.opt_strs("html-in-header").as_slice(), 
                                              matches.opt_strs("html-before-content").as_slice(), 
                                              matches.opt_strs("html-after-content").as_slice()) {
     Some(ef) => ef,
     None => return 3,
};

// ...
        (false, true) => return markdown::render(input, output.unwrap_or(Path::new("doc")),
                                                 &matches, &external_html),

// ...

            match html::render::run(krate, output.unwrap_or(Path::new("doc")), &external_html) {

// ...

Then render::run would change to take an ExternalHtml and Layout would gain an external_html field.

If you have any questions, just ask (here or IRC (huon)).

[zzmp]

I just responded to all your line comments, only to see that you had the same concerns when I finally got to your final comment :)

I'll go over this more thoroughly tomorrow, and I'll be sure to reach out if I have more questions.

[huonw]

It does not update the man rustdoc page, nor does it update the documentation in the rustdoc manual.

If you could, it would be good to update them (man/rustdoc.1 and src/doc/rustdoc.md respectively), but I'm happy to do them later.

(At the very least, you should change the argument names in rustdoc.md when you change these ones, so that it's not incorrect, even if it doesn't describe the full changes you have made.)

[zzmp]

I can do them when I fix everything else up. I really just could not find them.

[zzmp]

@huonw, I believe I've changed everything discussed.

This exposes more of rustdoc, in exposing functions and a macro that convert an array of strings (filepaths) to a string. It is in the rustdoc crate, so I don't think it is an issue.

I've also changed how the header/footer are generated. There are now explicit sections in all generated documentation for the header/footer, which are left blank if none are provided through the flags (but not removed from the DOM). They are given 0 padding in main.css, so as not to change the appearance if not included.
This was done so that they would always fall in the same column as the documentation, and never in the column of the cratelist. It was a deliberate decision, but is definitely opinionated.
The footer also appears at the bottom of the documentation, not at the bottom of the page. If the crate list is longer than the docs, the footer will appear below the docs (and not the crate list).

I can squash this into one commit, but wanted to leave it separate to make it easier to review. I can squash it if you'd like.

[huonw]

These need to be String with load returning Option<ExternalHtml>: None represents an error, and so rustdoc should exit.

(See the code in my large comment.)