Add nested sections

[hmelder]

Hi,

this pull request adds nested sections and some unit tests.

[coveralls]

Coverage decreased (-0.6%) to 86.638% when pulling 22a3982 on hmelder:nested_sections into 4047d66 on bmaupin:main.

[hmelder]

Coverage decreased (-0.3%) to 86.903% when pulling b567f79 on hmelder:nested_sections into 4047d66 on bmaupin:main.

Thats probably the func (e *ParentDoesNotExistError) Error() string method

[bmaupin]

I gave it a quick skim, looks good:

• Doesn't modify any existing API methods
• Adds a new method
• Added tests (thanks!)
• Looks like go fmt needs to be run, not a big deal

@owulveryck Would you like to take a look?

Thanks!

[bmaupin]

@hmelder Thanks so much for your contribution! I'm sorry it's taken me so long to take a look at this.

Unfortunately I found a couple issues with this PR:

• I called AddSubSection without internalFilename, and it used the next auto incrementing name. However, when I call AddSection after, it re-uses the same name. For example:
  1. Call AddSection, it uses the filename section0001.xhtml
  2. Call AddSubSection, it uses the filename section0002.xhtml (good so far)
  3. Call AddSection, it uses the filename section0002.xhtml
• I then tried AddSubSection with a different filename and a title

  • The subsection file appears in the generated EPUB

  • However, EPUBCheck gives the following warning:

    WARNING(OPF-003): My EPUB.epub(-1,-1): Item "EPUB/xhtml/subsection.xhtml" exists in the EPUB, but is not declared in the OPF manifest.

    And sure enough, the subsection doesn't appear in the TOC when I open the EPUB with Calibre's EPUB viewer

Could you please?:

1. Fix the above issues
2. Add a call to AddSubSection in testEpubValidity (in epub_test.go) to make sure that it will get validated by EPUBCheck? I'll add this to the contribution guidelines as well since it's probably a good rule of thumb.
3. (Optionally) run go fmt on the code when you're done so that the new code will match the formatting of the old code

In case it helps, this is the code I used to test it:

$ git diff
diff --git a/epub_test.go b/epub_test.go
index f040b23..b6c54fa 100644
--- a/epub_test.go
+++ b/epub_test.go
@@ -22,7 +22,7 @@ import (
 
 const (
        // Set this to false to not delete the generated test EPUB file
-       doCleanup             = true
+       doCleanup             = false
        testAuthorTemplate    = `<dc:creator id="creator">%s</dc:creator>`
        testContainerContents = `<?xml version="1.0" encoding="UTF-8"?>
 <container version="1.0" xmlns="urn:oasis:names:tc:opendocument:xmlns:container">
@@ -783,12 +783,17 @@ func testEpubValidity(t testing.TB) {
        e := NewEpub(testEpubTitle)
        testCoverCSSPath, _ := e.AddCSS(testCoverCSSSource, testCoverCSSFilename)
        e.AddCSS(testCoverCSSSource, "")
-       e.AddSection(testSectionBody, testSectionTitle, testSectionFilename, testCoverCSSPath)
+       testSectionPath, _ := e.AddSection(testSectionBody, testSectionTitle, testSectionFilename, testCoverCSSPath)
+       fmt.Printf("testSectionPath: %v\n", string(testSectionPath))
+
+       testSubSectionPath, _ := e.AddSubSection(testSectionPath, testSectionBody, "Test subsection", "subsection.xhtml", "")
+       fmt.Printf("testSubSectionPath: %v\n", string(testSubSectionPath))
 
        e.AddFont(testFontFromFileSource, "")
        // Add CSS referencing the font in order to validate the font MIME type
        testFontCSSPath, _ := e.AddCSS(testFontCSSSource, testFontCSSFilename)
-       e.AddSection(testSectionBody, testSectionTitle, "", testFontCSSPath)
+       testSection2Path, _ := e.AddSection(testSectionBody, testSectionTitle, "", testFontCSSPath)
+       fmt.Printf("testSection2Path: %v\n", string(testSection2Path))
 
        testImagePath, _ := e.AddImage(testImageFromFileSource, testImageFromFileFilename)
        e.AddImage(testImageFromFileSource, testImageFromFileFilename)

Thanks!

[hmelder]

Sorry for the inconvenience, I'll update the code in the following days :)

[hmelder]

Updated the code and fixed all outstanding issues.

[bmaupin]

I tested:

• AddSubSection with internalFilename (I just used the test you added)
• AddSubSection without internalFilename
• commenting out AddSubSection from testEpubValidity

In all cases, epub validation passed and the resulting epub looked as expected in an EPUB viewer.

Great job, thanks!

[owulveryck]

Sorry I think that will be hard for me to have a lot until next week. Le mar. 24 mai 2022 à 18:46, bmaupin ***@***.***> a écrit :

…

I gave it a quick skim, looks good: - Doesn't modify any existing API methods - Adds a new method - Added tests (thanks!) - Looks like go fmt needs to be run, not a big deal @owulveryck <https://github.com/owulveryck> Would you like to take a look? Thanks! — Reply to this email directly, view it on GitHub <#58 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACYL7KOZDQUY4RNYPCOH5UTVLUBWXANCNFSM5WJG4FTA> . You are receiving this because you were mentioned.Message ID: ***@***.***>