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

Fixed version of #1346: Allow split_by to accept numeric values #1490

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

katrinabrock
Copy link
Contributor

@katrinabrock katrinabrock commented Feb 6, 2025

Close #1346

katrinabrock and others added 4 commits February 5, 2025 15:59
Squashed commit of the following:

commit 99f8698
Author: Laure Cougnaud <[email protected]>
Date:   Tue Jun 14 10:33:16 2022 +0000

    limit numeric split_by to: [1:6] + update documentation

commit 3304d5a
Merge: 7f1e38b 4cdf9c6
Author: Laure Cougnaud <[email protected]>
Date:   Tue Jun 14 08:20:17 2022 +0000

    merge branches

    Merge remote-tracking branch 'OAGithubRStudio/main' into split_by-numeric

    # Conflicts:
    #	R/gitbook.R
    #	R/html.R
    #	man/build_chapter.Rd
    #	man/epub_book.Rd
    #	man/gitbook.Rd
    #	man/html_chapters.Rd
    #	man/html_document2.Rd
    #	man/pdf_book.Rd
    #	man/publish_book.Rd
    #	man/render_book.Rd
    #	man/resolve_refs_html.Rd
    #	man/serve_book.Rd

commit 7f1e38b
Author: lcougnaud <[email protected]>
Date:   Mon Nov 4 11:03:18 2019 +0100

    fix for nested subsection (part 1)

commit ee342c7
Author: lcougnaud <[email protected]>
Date:   Thu Oct 31 16:17:33 2019 +0100

    fix specification split_by

commit 60bb103
Author: lcougnaud <[email protected]>
Date:   Thu Oct 31 14:25:09 2019 +0100

    fix inclusion subsections

commit 9c11691
Author: lcougnaud <[email protected]>
Date:   Thu Oct 24 18:05:31 2019 +0200

    split_chapters: fix numbering higher level sections that split_by

commit 0442817
Author: lcougnaud <[email protected]>
Date:   Thu Oct 24 17:42:57 2019 +0200

    update split_chapters for numeric 'split_by'
@katrinabrock katrinabrock changed the title Split by rebased Fixed version of #1346: Allow split_by to accept numeric values Feb 6, 2025
@katrinabrock
Copy link
Contributor Author

Same content as #1346 but with the failing tests fixed :-)

@katrinabrock
Copy link
Contributor Author

You can cleanly cherry pick every thing after the add tests... commit on to @lcougnaud 's PR if that's better for giving them proper credit than merging this one.

@katrinabrock
Copy link
Contributor Author

@yihui The code in this one is ready for review btw. Not sure if that was clear from my above comments.

The commit messages certainly leave something to be desired. I can fix them and make sure @lcougnaud is show as the author of the code they wrote if that matters. Let me know. I figured those messages would get squashed away anyhow.

@lcougnaud
Copy link

@katrinabrock thanks for fixing the tests & finalizing the changes! For me it is all fine to squash everything into one single commit during the merging.

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I feel this is beyond my capability to review, and I'll simply trust your hard work. Before I merge it, could you test a couple of books and see if they render to the same HTML files before/after this PR? For example:

You can first render a book without this PR, temporarily commit the HTML output files in Git, and then install bookdown with this PR (and restart R), render again. Then see if there are any changes in Git.

Thanks!

@katrinabrock
Copy link
Contributor Author

ok! yes, I can do that. I will look also take closer at the code written by @lcougnaud before you blindly merge so at least there are two sets of eyes on it.

@yihui
Copy link
Member

yihui commented Feb 20, 2025

Great! If you run into any errors when building these books, you can add eval = FALSE to the code chunks that produced the errors (or even delete them).

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.

[FR] Support split_by for section level higher than 2 (section) in gitbook
3 participants