[
https://issues.apache.org/jira/browse/FOP-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13937571#comment-13937571
]
Vincent Hennebert commented on FOP-2293:
----------------------------------------
Hi Seifeddine,
thanks for your patch. I've applied it in [rev.
1578270|http://svn.apache.org/r1578270]. I made the following adjustments:
* The BestFitGlue class, and the special implementation of handleGlueAt in
PageBreakingLM are not necessary. I'm actually not too sure how it was supposed
to work: the method was iterating over all the nodes for the previous page and
making a decision based on the state of one of them, without being sure that
it's the one that would have ultimately been selected. Anyway, the code works
with a normal glue whose width matches the width of the first variant. Or did I
miss your intent?
* Not sure why you set the penalty value of a BestFitPenaly to -1? I left it to
0.
* I left the KnuthVariant class inside BestFitPenalty and left its name
unchanged. The reason is that the layoutmgr package is already overcrowded and
BestFitPenalty provides a nice encapsulation. Also, it's not really related to
the Knuth algorithm so having Knuth in its name didn't seem all that relevant
to me.
* I integrated the test cases you provided. I made some modifications to
simplify them and make them more complete.
This is taking really good shape now. Here's a check list of things to be done
before merging back the branch to trunk:
* Documentation, in the form of a patch against the web site. The best place
probably is the Extensions page under the trunk tab. See the [CMS
Reference|https://www.apache.org/dev/cmsref.html] to find out how to modify the
website, and especially the
[process|https://www.apache.org/dev/cmsref.html#non-committer] for
non-committers.
* Improve test coverage: I think the layout tests we already have are pretty
good, but one cannot be sure until test coverage has been checked. You can use
your IDE's coverage tool or run JaCoCo on the command line: ant -f jacoco.xml
coverage-report.
* Code clean-up (this patch was already a good start):
** The recommendation says that fo:multi-switch and fo:multi-case do not
generate any areas. The corresponding LayoutManager classes should be modified
to not create instances of Block, but instead directly return their kids'
areas. I did notice indeed that a lot of nested blocks were appearing in the
area trees created out of the layout tests.
** The BestFitLayoutUtils class should probably be removed and its content
moved into MultiSwitchLM, as IIC it's used only by that latter.
** It's getting time to sort out the remaining TODOs.
** Run Checkstyle, and possibly FindBugs.
** Finally, check the diff against trunk, which will allow you to easily spot
spurious changes that can be cleaned up.
Thanks for your efforts!
Vincent
> Whitespace management extension
> -------------------------------
>
> Key: FOP-2293
> URL: https://issues.apache.org/jira/browse/FOP-2293
> Project: Fop
> Issue Type: New Feature
> Components: general
> Affects Versions: trunk
> Reporter: Seifeddine Dridi
> Priority: Minor
> Labels: XSL-FO
> Fix For: trunk
>
> Attachments: FO_multi-switch_best-fit_ext_rev2.patch,
> FO_multi-switch_best-fit_ext_rev3.patch,
> FO_multi-switch_best-fit_ext_rev4.patch,
> FO_multi-switch_best-fit_ext_rev5.patch,
> FO_multi-switch_best-fit_ext_rev6.patch,
> FO_multi-switch_best-fit_ext_rev7.patch,
> FO_multi-switch_best-fit_ext_rev8.patch, FO_multi-switch_test.fo,
> FO_multi-switch_with_best-fit_extension.patch, bestfit.fo, doc.pdf,
> multi-switch-testcases.zip, multi-switch_bestfit.fo,
> multiple-feasible-nodes.fo, patch-rev1.1.patch, patch-rev1.patch,
> patch-rev2.patch, patch.patch, two-valid-variants.fo
>
>
> I have been working on an extension for whitespace management, similar to
> what's described here:
> http://wiki.apache.org/xmlgraphics-fop/WhitespaceManagement
> The logic of the extension is very simple: the user defines a set of
> alternatives that he wishes to insert at the end of a page, then if there is
> enough space left, FOP will pick the alternative that best matches the user's
> selection criteria (first fit, smallest fit, biggest fit).
> This is my first work on FOP and it took me almost 2 months to reach this
> stage in development. But it's not the end of course, so I'm relying on your
> feedback to improve it.
> Thank you
--
This message was sent by Atlassian JIRA
(v6.2#6252)
