DO NOT REPLY [Bug 50725] KnuthSequence documentation?

2011-02-07 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=50725

--- Comment #2 from Simon Pepping spepp...@apache.org 2011-02-07 03:22:51 EST 
---
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 ListKnuthSequence,
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.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.


Re: svn commit: r1067533 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java

2011-02-07 Thread Simon Pepping
I am pleased that you undertake this kind of cleanup work in the
layout engine. It is very useful that you try to make this piece of
code more accessible.

Simon

On Sun, Feb 06, 2011 at 12:18:15AM +0100, Andreas Delmelle wrote:
 On 05 Feb 2011, at 22:49, adelme...@apache.org wrote:
 
  Author: adelmelle
  Date: Sat Feb  5 21:49:58 2011
  New Revision: 1067533
  
  URL: http://svn.apache.org/viewvc?rev=1067533view=rev
  Log:
  Code cleanup
  
  Modified:
 
  xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
 
 In preparation of decommissioning its StackingIter, I noticed I had some 
 minor cleanups ready for this class. Checking closer, the code was still way 
 too messy for my taste, so I went further --and further... Quite far, 
 actually, so a bit of explanation needed in case someone goes looking for 
 code that I removed.
 
 I noticed there was quite a lot of code that, in fact, was never, ever used. 
 It seemed like an experiment from Luca, that likely should have been kept in 
 a branch instead of being committed to the trunk in an incomplete state. All 
 it did here was confuse people, in a class which is quite heavily used.
 I first spent quite some time cleaning up commented log.debug() statements in 
 the method createUnitElements(), then decided to check where the method was 
 called, and found it was only used in one place, in getChangedKnuthElements():
 
  -/* estensione: conversione complessiva */
  -/*LF*/  if (bpUnit  0) {
  -/*LF*/  storedList = returnedList;
  -/*LF*/  returnedList = createUnitElements(returnedList);
  -/*LF*/  }
 
 Now, that bpUnit member is written *only* in BlockLayoutManager and 
 BlockContainerLayoutManager, where it is set to 0 in the initialize() method. 
 In effect, the method was unused, so I decided to bite the bullet and remove 
 it.
 
 Additionally, given the above, I have also removed similar references to this 
 bpUnit member in other places, which eliminates some conditional branches. I 
 have not yet removed the variable itself, since it is still read in a few 
 subclasses.
 
 
 Regards,
 
 Andreas
 ---
 


Re: svn commit: r1067533 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java

2011-02-07 Thread Adrian Cumiskey
I'd like to second Simon's comments, very unselfish and good work Andreas.

Adrian.

On 7 February 2011 16:26, Simon Pepping spepp...@leverkruid.eu wrote:

 I am pleased that you undertake this kind of cleanup work in the
 layout engine. It is very useful that you try to make this piece of
 code more accessible.

 Simon

 On Sun, Feb 06, 2011 at 12:18:15AM +0100, Andreas Delmelle wrote:
  On 05 Feb 2011, at 22:49, adelme...@apache.org wrote:
 
   Author: adelmelle
   Date: Sat Feb  5 21:49:58 2011
   New Revision: 1067533
  
   URL: http://svn.apache.org/viewvc?rev=1067533view=rev
   Log:
   Code cleanup
  
   Modified:
  
  
 xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
 
  In preparation of decommissioning its StackingIter, I noticed I had some
 minor cleanups ready for this class. Checking closer, the code was still way
 too messy for my taste, so I went further --and further... Quite far,
 actually, so a bit of explanation needed in case someone goes looking for
 code that I removed.
 
  I noticed there was quite a lot of code that, in fact, was never, ever
 used. It seemed like an experiment from Luca, that likely should have been
 kept in a branch instead of being committed to the trunk in an incomplete
 state. All it did here was confuse people, in a class which is quite heavily
 used.
  I first spent quite some time cleaning up commented log.debug()
 statements in the method createUnitElements(), then decided to check where
 the method was called, and found it was only used in one place, in
 getChangedKnuthElements():
 
   -/* estensione: conversione complessiva */
   -/*LF*/  if (bpUnit  0) {
   -/*LF*/  storedList = returnedList;
   -/*LF*/  returnedList = createUnitElements(returnedList);
   -/*LF*/  }
 
  Now, that bpUnit member is written *only* in BlockLayoutManager and
 BlockContainerLayoutManager, where it is set to 0 in the initialize()
 method. In effect, the method was unused, so I decided to bite the bullet
 and remove it.
 
  Additionally, given the above, I have also removed similar references to
 this bpUnit member in other places, which eliminates some conditional
 branches. I have not yet removed the variable itself, since it is still read
 in a few subclasses.
 
 
  Regards,
 
  Andreas
  ---
 



DO NOT REPLY [Bug 50725] KnuthSequence documentation?

2011-02-07 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=50725

--- Comment #3 from Andreas L. Delmelle adelme...@apache.org 2011-02-07 
14:02:52 EST ---
(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...

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.