brailleapps / dotify.formatter.impl

Provides an implementation of the formatter interfaces in dotify.api
GNU Lesser General Public License v2.1
0 stars 6 forks source link

Various changes to <evaluate/> #99

Closed bertfrees closed 5 years ago

bertfrees commented 5 years ago
bertfrees commented 5 years ago

I have rebased this onto master. See the corresponding commits in https://github.com/brailleapps/dotify.formatter.impl/compare/master...sbsdev:super/sbs.

kalaspuffar commented 5 years ago

Hi @bertfrees

I've prepared this PR to run regression tests as the changes we discuss is of minor nature. In order to merge this one though we need to rebase it and handle all the code style issues. I got 9 instances of missing curly braces when I tried to build this PR.

Best regards Daniel

bertfrees commented 5 years ago

I got 9 instances of missing curly braces when I tried to build this PR.

Fixed now. And I rebased to branch onto master.

bertfrees commented 5 years ago

@kalaspuffar I added commit 67da750 which fixes the regression in the book that you sent me. I hope it will fix the 10 other books too. Unfortunately I can't explain how the regression was caused and why my commit would have any effect. First I thought that it had something to do with pages that are produced (by PageSequenceBuilder2) but do not end up in the volume. But think this should not be the problem because of the crh.setReadOnly() and crh.setReadWrite() calls in VolumeProvider#nextBodyContents before/after the "find" phase, and in the "split" phase no pages are produced that will not end up in the volume.

Note that I rebased the branch.

kalaspuffar commented 5 years ago

Hi @bertfrees

Except for the Exceptions and the addition of better error messages, this is ready for merge. Are you going to make any improvements to mine or @PaulRambags suggestions or do we consider this done?

Best regards Daniel