https://issues.apache.org/bugzilla/show_bug.cgi?id=47359





--- Comment #8 from Nicolas Socheleau <[email protected]>  
2009-06-16 09:04:59 PST ---
> Batik uses the space character for indenting. Please update your patch (which
> uses a mix of tabs and spaces) with this in mind. ;-)
> 
> 
> Missing space before "asb" in:
> 
> +        fillAttributedStringBuffer(ctx, element, true, null, null, null,
> null,asb);
> 
> 
> Missing space before "elementAttributes" in:
> 
> +    parentAttributes =
> handleNestedBaselineShift(parentAttributes,elementAttributes);
> 
> 

I've addressed those issues in the 2nd submitted patch.

> Is it safe to assume "elementAttributes" is always non-null? That is, should 
> we
> be making the same null check made for the "parentAttributes" nearby?
> 
> +        if ( elementAttributes.containsKey(BASELINE_SHIFT) )
> 

handleNestedBaselineShift( Map, Map ) is called from one location only and a
few lines before the call, elementsAttributes is set to new HashMap(). I assume
it is safe not to check elementAttributes within that method.

> 
> I'd say the space before the semi-colon could be dropped in:
> +                    for(int k = 0 ; k < values.size(); k++)
> 
> 
> Although this portion was adapted from previous code, I'd propose that spaces
> were inserted between "==" and also the "+= -" was changed to "-=" in:
> 
> +                            if
> (baselineValue==TextAttribute.SUPERSCRIPT_SUPER) {
> +                                baselineAdjust += baselineAscent*0.5f;
> +                            } else if
> (baselineValue==TextAttribute.SUPERSCRIPT_SUB) {
> +                                baselineAdjust += -baselineAscent*0.5f;
> 
> 

adjusted in the latest patch

> With these small changes addressed, I'd say the patch is ready for review. ;-)
> 
> 
> Apart from the changes, I'd just like to leave a couple of thoughts which I
> haven't been able to dig down properly (at least, not for now):
>  * Is this code ready for on-the-fly modification of the baseline-shift
> property? By scripting or by placing a SMIL animation on the property, either
> in the child or in the parent for a nesting sample, does the proposal keep up
> with?

I've attached test file for animation and scripting as well for the review.

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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to