On 07 Jul 2009, at 17:22, Vincent Hennebert wrote:
Ok. To be honest I don’t remember why I did that. Probably a cheap
attempt to make a failing test case with footnotes pass again. At any
rate this will have to be checked. I’m wondering what’s supposed to
happen if there is no legal break in the sequence.
I think it was necessary because the keep-implementation possibly
inserts penalties with value INFINITE, but those may still be
considered as a legal break. If the penalty corresponds to a
keep.within-page, the breakpoint may still be considered as a mere
column-break.
BTW: I've considered your suggestion of using penalties with value 0,
but the downside seems to be that those would always be considered as
a legal break by the default implementation in BreakingAlgorithm.
For penalties corresponding to page-, or column-keeps I have relocated
the special treatment to PageBreakingAlgorithm.handlePenaltyAt(), so
where you had initially added a check 'breakClass != EN_LINE', the
additional method now allows to move that check to the class where it
belongs.
- return bestActiveNode.line;
+ return (bestActiveNode == null) ? -1 :
bestActiveNode.line;
Why did you add this test? Did you run into NPE with some test
cases?
Not yet, but IntelliJ IDEA always marks dereferences that possibly
lead
to NPEs as a warning. Better safe than sorry, I guess...
I don’t agree. The check should be added only if it corresponds to
a valid situation that may occur when calling this method. Otherwise
it
should be replaced with an earlier ‘assert bestActiveNode != null’.
And
given that, in the calling findBreakingPoints method, there is a
special
return statement when the number of active nodes falls to 0, I suspect
that we are in the latter case.
Makes sense. I'll take care of changing it to an 'assert'.
Regards
Andreas Delmelle
e-mail: andreas.delmelle.AT.telenet.be
Skype: adlm0608
Jabber: mandr...@jabber.org
