DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUGĀ·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=40271>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED ANDĀ·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=40271





------- Additional Comments From [EMAIL PROTECTED]  2006-09-08 09:27 -------
I've reviewed the latest patch. Here are my comments, some of which I already
sent to Patrick when we chatted earlier via Skype:
- in the testcase an overflow happens in the second column because border widths
were not respected for calculating the column widths.
- The resulting widths don't seem right. The first column should be wider, the
third narrower to provide a more balanced result.
- The variable names as mentioned by Andreas are still not changed. I don't
particularly like names which contain indexes. Speaking names are always
preferrable. I think there's room for improvement.
- Your Java style is a little inconsistent. Indents are 4 spaces, there's a
space after "if", a space before and after "==" etc. You might want to install a
CheckStyle plugin. It could help you avoid such things by telling you where the
problems are. If you could improve that a little, it will cause less work for us
when we apply the patch.
- System.out.println calls have to be changed to use log.*(). Use
log.is*Enabled() if you construct Strings with variables for log levels of info
or higher.
- The main point: As you know I'm still unhappy that the
TableContentLayoutManager (with all its children) is constructed twice. If this
can be isolated for the auto-table-layout case (i.e. the fixed table layout is
not affected) it is somewhat acceptable in the short-term but the next big goal
has to be to split the element list generation from the line-breaking.
- It would be good to have additional test cases showing stuff that does not
work, yet, like a table with some fixed-width columns.
- In the end, the message "table-layout="auto" is currently not supported by
FOP" can be removed. :-)

I'm happy to see that we're on the way on this topic. And it is good to see the
discussion about the splitting of the element list generation from the line
breaking.
Unfortunately, I don't think the patch is ready to be applied, yet. What do the
others think?

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

Reply via email to