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

Options #27

Merged
merged 9 commits into from
Feb 21, 2020
Merged

Options #27

merged 9 commits into from
Feb 21, 2020

Conversation

gambhiro
Copy link
Contributor

@gambhiro gambhiro commented Feb 19, 2020

Hello,

This includes two options I needed, cover-image and additional-resources.

A rough outline of my procedure was:

  • start this branch options from master
  • merge your work from the template branch
  • resolve conflicts
  • add the options to specify cover-image and additional-resources

So this includes your work in the Handlebars template branch, with small patches to make it compile, which is effectively PR #8 and PR #26.

It builds an EPUB which passes epubcheck and can be used with kindlegen to generate a MOBI successfully like so:

kindlegen "./book.epub" -dont_append_source -c1 -verbose

When I was resolving the merge conflicts, there was one thing I didn't understand and marked with a FIXME in generator.rs.

Building the epub worked for me without it, but you would probably remember whether it is still relevant.

        /* FIXME: is this logic still necessary?
        // unfortunately we need to do two passes through `ch.sub_items` here.
        // The first pass will add each sub-item to the current chapter's toc
        // and the second pass actually adds the sub-items to the book.
        for sub_item in &ch.sub_items {
            if let BookItem::Chapter(ref sub_ch) = *sub_item {
                let child_path = sub_ch.path.with_extension("html").display().to_string();
                content = content.child(TocElement::new(child_path, format!("{}", sub_ch)));
            }
        }
        */

@Michael-F-Bryan
Copy link
Owner

I think we needed it because I had issues with items inside nested chapters (or maybe items inside items inside chapters) not showing up in the table of contents. If it works fine for you I'd say it's okay to remove.

You can bump the MSRV in travis.yml to about 1.40.0 to fix the CI error. You may want to add a comment referencing rust-lang/cargo#7579 so we know why that particular version was picked.

Other than that I'm happy to merge the code and make a release.

@gambhiro
Copy link
Contributor Author

gambhiro commented Feb 20, 2020

Thanks for the notes.

I ran a quick test just now with a nested chapter structure, and they showed up in the TOC and rendered correctly.

@Michael-F-Bryan Michael-F-Bryan merged commit d310138 into Michael-F-Bryan:master Feb 21, 2020
@gambhiro gambhiro mentioned this pull request Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants