Created attachment 18956 [details]
Corrects Logger in several LM classes

Corrects (adds where missed) Logger from protected to private in these classes:

LineLayoutManager, TableCellLayoutManager, TableLayoutManager,
BreakingAlgorithm, PageBreakingAlgorithm

Andrejus,

Thanks for your patch. I have a few comments.

1. Your patch for src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
reads:

     /** the logger for the class */
     protected static Log classLog = LogFactory.getLog(PageBreakingAlgorithm.class);
 
+    /** the logger for the instance */
+    private static Log log = classLog;

Why don't you remove 'protected static Log classLog'?

Re the comment, a static variable is for the class, not for the instance.

2. In the LayoutManagers, there is a protected static Log instance in
org.apache.fop.layoutmgr.AbstractBaseLayoutManager, assigned to class LayoutManager:

protected static Log log = LogFactory.getLog(LayoutManager.class)

This is used by all LayoutManagers. I can imagine that you find this too coarse,
and prefer to see a different logger for each LayoutManager, but then it would
be better to do so for each LayoutManager.

Regards, Simon

Hi Simon,

I prefer to touch others code in very kind matter, meaning that I'd like to 
see as small recompiling as possible (in this case I wanted to touch just log 
members and not classLog I had no knowledge about). Yes, there is next step to 
do to change all other protected Loggers for LMs, but for me it would be much 
easier to to if we would deal with code tagging/modules/SVN branches instead 
of direct text patches. Right now I've applied Patrick's patch and to change 
other Loggers to private as well I would need once again to checkout whole 
source tree to another project and make those changes there. Actually I've 
planned finish that after I'll get some understanding of Patrick's code as 
well as with LMs invocations.

Sorry if it seems incomplete for you right now, but I've tried to make code 
after changes to be compilable.

Andrejus

(In reply to comment #3)

> members and not classLog I had no knowledge about). Yes, there is next step to 
> do to change all other protected Loggers for LMs, but for me it would be much 
> easier to to if we would deal with code tagging/modules/SVN branches instead 
> of direct text patches. Right now I've applied Patrick's patch and to change 

Indeed, doing this by patches is bothersome. But as it is, the patch is a bit
small and random. I would prefer it if you would make the required changes in
your working copy and submit them in a patch which contains more work.

Of course, I could make the changes to the LM tree myself. What do the FOP
committers think: Give each LM its own logger? It will provide more clarity
during debugging, but it will also increase the number of loggers considerably.

Simon

(In reply to comment #4)
> Of course, I could make the changes to the LM tree myself. What do the FOP
> committers think: Give each LM its own logger? It will provide more clarity
> during debugging, but it will also increase the number of loggers considerably.

+1 from me. It bugs me every now and then but so far I've never had enough
energy to actually do something about it. I only changed something for the
breaking algorithm where it was essential for me to keep line break stuff apart
from page break stuff while debugging.

Patch extended to all LMs which use a logger and applied. Andrejus, thanks for
your patch.

batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed