xmlgraphics-fop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vincent Hennebert <vhenneb...@gmail.com>
Subject Re: DO NOT REPLY [Bug 46315] fox:needs-balancing extension
Date Tue, 02 Dec 2008 11:01:00 GMT
Hi Georg,

[Forwarding to fop-dev, I don’t think there’s anything that needs to be
kept secret ^^ ]

Georg Datterl wrote:
> Hi Vincent,
> 
> Thanks for the help.
> 
>> - you can't play with LayoutContext in that way. The value of the property needs

>> to be known before Knuth elements are added to the element list (I don't want to

>> enter the details too much). Your best bet is to mimic the way the span property

>> is handled, see FlowLayoutManager in the attached patch
> 
> That was the step I was missing all the time.
> 
>> Next step:
>> - clean up a bit
>> - make the modifications adhere to FOP's standard (see checkstyle-4.0.xml at the
root of the project)
> 
> Checkstyle says Processing xml and Loading stylesheet. I assume, that's a good sign...

Actually that would be easier for you if you could set it up in your
development environment (which one are you using? You can have a look
there: http://wiki.apache.org/xmlgraphics-fop/FOPIDESetupGuide). There
are currently something like 1,900 checkstyle warnings in the code base.
Obviously you would have to fix only the warnings that appear on the
lines you modified, and it’s easier done within an IDE.

As an alternative you can run ‘ant checkstyle’ on the command line, but
then you will have to search through the results.


>> - add a test case for the new feature (see the test/layoutengine/standard-testcases/
directory)
> 
> Can I use the testcase I sent to you or only half of it or is that too big? 
> Or is it too small and should be a third flow without a value to test default value?

It’s almost alright, just slightly big. You could reduce the page height
(no need to have a standard page size) and use two page-sequences: one
to test the default value like you say above, one that activates the
extension; in the latter you could have two spanning blocks, one
occurring ‘on the first column’, one on the second. You can use the very
same content for both page sequences, just modifying the extension
property. Filling the  blocks with ‘AAA’, ‘BBB’, etc. is a very good
idea.

As a next step we also need to discuss the name of the property. Since
this is an extension specific to FOP it needs to be in the ‘fox:’
namespace. Also, maybe it would be slightly more intuitive to give it
a name such that the default value would be false, and that you have to
explicitly set it to true to override the default behaviour. So
something like fox:do-no-balance-columns-before... but much more
concise!


Thanks,
Vincent

> Regards,
>  
> Georg Datterl
>  
> ------ Kontakt ------
>  
> Georg Datterl
>  
> Geneon media solutions gmbh
> Gutenstetter Straße 8a
> 90449 Nürnberg
>  
> HRB Nürnberg: 17193
> Geschäftsführer: Yong-Harry Steiert 
> 
> Tel.: 0911/36 78 88 - 26
> Fax: 0911/36 78 88 - 20
>  
> www.geneon.de
>  
> Weitere Mitglieder der Willmy MediaGroup:
>  
> IRS Integrated Realization Services GmbH:    www.irs-nbg.de 
> Willmy PrintMedia GmbH:                            www.willmy.de
> Willmy Consult & Content GmbH:                 www.willmycc.de 
> -----Ursprüngliche Nachricht-----
> Von: bugzilla@apache.org [mailto:bugzilla@apache.org] 
> Gesendet: Montag, 1. Dezember 2008 13:19
> An: Georg Datterl
> Betreff: DO NOT REPLY [Bug 46315] fox:needs-balancing extension
> 
> https://issues.apache.org/bugzilla/show_bug.cgi?id=46315
> 
> 
> Vincent Hennebert <vhennebert@gmail.com> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>   Attachment #22970|0                           |1
>         is obsolete|                            |
> 
> 
> 
> 
> --- Comment #2 from Vincent Hennebert   2008-12-01 04:18:44 PST --- 
> Created an attachment (id=22971)  --> 
> (https://issues.apache.org/bugzilla/attachment.cgi?id=22971)
> Version 1, modified to basically do the job
> 
> Hi Georg,
> 
> Thanks for the patch. This is basically what needs to be done. I attach a modified version
of your patch with the following comments:
> - you don't need to do anything on the Flow object actually. Since the property is defined
as inherited, the property sub-system will take care of this automatically.
> - it's best to move the definition of the property from the createBlockAndLineProperties
method to createLayoutProperties (where the span property is also defined)
> - you can re-use the genericBoolean field in FOPropertyMapping
> - the default value is true and not inherit. The inherit characteristic is defined separately
> - you can't play with LayoutContext in that way. The value of the property needs to be
known before Knuth elements are added to the element list (I don't want to enter the details
too much). Your best bet is to mimic the way the span property is handled, see FlowLayoutManager
in the attached patch
> 
> Please have a look at the attached patch; it worked for me on a basic example but more
extensive testing is needed. To ensure it doesn't break anything you can run 'ant junit' on
the command line at the base of the project. It will run all the test and print a big fat
warning in case one is broken.
> 
> Next step:
> - clean up a bit
> - make the modifications adhere to FOP's standard (see checkstyle-4.0.xml at the root
of the project)
> - add a test case for the new feature (see the test/layoutengine/standard-testcases/
directory)
> 
> Thanks!
> Vincent
> 
> 
> --
> Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
> ------- You are receiving this mail because: ------- You reported the bug.

Mime
View raw message