Bug 50725 - KnuthSequence documentation?
Summary: KnuthSequence documentation?
Status: NEEDINFO
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: page-master/layout (show other bugs)
Version: trunk
Hardware: All All
: P3 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-06 12:17 UTC by Andreas L. Delmelle
Modified: 2012-04-24 05:32 UTC (History)
0 users



Attachments
proposed patch to deal with TODO, uncovers to a new one... (4.81 KB, application/octet-stream)
2011-02-06 12:17 UTC, Andreas L. Delmelle
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas L. Delmelle 2011-02-06 12:17:10 UTC
Created attachment 26614 [details]
proposed patch to deal with TODO, uncovers to a new one...

Attached is a patch against fop.layoutmgr.KnuthSequence, which takes care of a TODO (Do not extend ArrayList) by implementing the List interface via proxy methods to a member list.

Reason this is not committed yet, is that I wanted to go a step further and add type safety to this class, but noticed that this is currently impossible. 
The explanation is also added in a new TODO: there is an inconsistency (or at least incongruency) between block-layout and inline-layout. Block-layout works mainly with KnuthSequences as List<ListElement>  where inline-layout seems to expect List<KnuthSequence>...

Suggestions? Ideas? I would like to see those aligned, and in theory, once KnuthSequence implements List, it is free to extend ListElement, if that would be appropriate. The other way around could also make sense: consider each ListElement as a KnuthSequence of a single element. Although that would require some more refactoring, since we have to extract the KnuthSequence/List interface before ListElement can implement it.
Comment 1 Andreas L. Delmelle 2011-02-06 12:32:53 UTC
Forgot to add: the key class, the single one that seems to be making it impossible to turn KnuthSequence into a List<ListElement>, is InlineLayoutManager. It's there, in the getNextKnuthElements() method, that you cannot make returnList a List<ListElement> without triggering a compile error.

If the issue there were solved, it would be possible to add type safety to the return type of LayoutManager.getNextKnuthElements(), and make it either a List<ListElement> or List<KnuthSequence>.
Comment 2 Simon Pepping 2011-02-07 03:22:51 UTC
I know. I created that to be able to work with block-level content in inline-level content. An inline LM may have block-level LMs in its subtree. I solved the problem by letting an inline-level LM receive List<KnuthSequence>, where each sequence of inline-level layout would be in one InlineKnuthSequence, and block-level layout would generate BlockKnuthSequences. At the time I did this, there were no generics yet. I have long been aware that this violates type safety, but I do not know a solution. LM.getNextKnuthElements is called for block and inline-level LMs alike, but the structure of their subtrees is different.
Comment 3 Andreas L. Delmelle 2011-02-07 14:02:52 UTC
(In reply to comment #2)

> At the time I did this, there were no generics yet. I have long been aware 
> that this violates type safety, but I do not know a solution. 
> LM.getNextKnuthElements is called for block and inline-level LMs alike, 
> but the structure of their subtrees is different.

Right, I remember now. Well, it's not too dramatic, and the issue of type safety mainly concerns fo:wrappers, IIC[*]. For the remainder, no biggie. Nobody would dream of adding, say, a LayoutManager to the returnList in getNextKnuthElements(), simply because it is obvious from the context what type of elements is expected. It's just a nice-to-have, if we would be able to define it in the interface. 

As for a solution, my proposal might just work fine. Given that, after the patch, KnuthSequence is no longer a subclass of ArrayList, it is free to become a special kind of ListElement (compound, rather than atomic). The only thing I still have to figure out, is whether that could be useful for other purposes, or whether that just adds more complexity and potential confusion...

[*] cfr. the ClassCastException issue that popped up a number of times in the past, and once fixed, it resurfaced further downstream (see also Bugzilla 47530). Fixed in FlowLM, then surfaced in BlockStackingLM, then triggered by a call to ElementListUtils...
Comment 4 Glenn Adams 2012-04-07 01:42:42 UTC
resetting P2 open bugs to P3 pending further review
Comment 5 Glenn Adams 2012-04-08 04:20:34 UTC
andreas, any progress on finalizing this patch?
Comment 6 Glenn Adams 2012-04-24 05:32:49 UTC
(In reply to comment #5)
> andreas, any progress on finalizing this patch?

??