Bug 46926 - [PATCH] More control over RasterizerTask output files
Summary: [PATCH] More control over RasterizerTask output files
Status: NEW
Alias: None
Product: Batik - Now in Jira
Classification: Unclassified
Component: SVG Rasterizer (show other bugs)
Version: 1.8
Hardware: All All
: P4 enhancement
Target Milestone: ---
Assignee: Batik Developer's Mailing list
URL: http://www.nabble.com/-PATCH--More-co...
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2009-03-27 07:00 UTC by Helder Magalhães
Modified: 2009-10-12 05:20 UTC (History)
1 user (show)



Attachments
Patch version 2 (16.88 KB, patch)
2009-03-27 08:10 UTC, Martin von Gagern
Details | Diff
Patch version 3 (19.35 KB, patch)
2009-04-22 07:22 UTC, Martin von Gagern
Details | Diff
Patch version 4 (19.50 KB, patch)
2009-04-28 07:13 UTC, Martin von Gagern
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helder Magalhães 2009-03-27 07:00:15 UTC
Originally reported by Martin von Gagern.

I've created this report as the original mailing list thread (report's URL) didn't get any follow up, to make sure this doesn't get lost, at least not without a proper debate. :-)

The proposed patch is available through the mailing list post.
Comment 1 Martin von Gagern 2009-03-27 08:10:39 UTC
Created attachment 23422 [details]
Patch version 2

To avoid one level of indirection, I'll attach my patch here.

I've also taken up Helder's vote for compatibility, and added a flatten attribute defaulting to true.
Comment 2 Helder Magalhães 2009-03-27 09:01:13 UTC
(In reply to comment #1)
> Created an attachment (id=23422) [details]
> Patch version 2

-        <javac srcdir="${src}" destdir="${build.classes}" />
+        <javac srcdir="${src}" destdir="${build.classes}" debug="true">
+            <classpath>
+                <pathelement location="../../classes"/>
+                <fileset dir="." includes="lib/**/*.jar"/>
+            </classpath>
+        </javac>

From the main "build.xml" file:

  If you are going to be doing development you may want to create a file
  called 'build.properties' with the line 'debug=on' in it.  This will 
  turn on the generation of debugging information when compiling Batik.

I'm not sure if it would be a good idea to make this here also but, at least, creating a property "debug" and using 'debug="${debug}"' instead (for coherency with the main build file) looked like a good idea. :-)


> I've also taken up Helder's vote for compatibility, and added a flatten
> attribute defaulting to true.

Great! :-)  Not breaking existing work is, IMHO, a good approach in general. ;-)


Finally, I believe the patch should also address some documentation regarding the newly introduced features. (The documentation source for that is "rasterizer.xml" [1] which, after published, should appear at "rasterizer.html" [2].

[1] documentation-sources/content/xdocs/tools/rasterizer.xml
[2] http://xmlgraphics.apache.org/batik/tools/rasterizer.html#task
Comment 3 Martin von Gagern 2009-04-22 07:22:14 UTC
Created attachment 23529 [details]
Patch version 3

(In reply to comment #2)
> From the main "build.xml" file:
> 
>   If you are going to be doing development you may want to create a file
>   called 'build.properties' with the line 'debug=on' in it.  This will 
>   turn on the generation of debugging information when compiling Batik.
> 
> I'm not sure if it would be a good idea to make this here also but, at least,
> creating a property "debug" and using 'debug="${debug}"' instead (for
> coherency with the main build file) looked like a good idea. :-)

Did that, and also loaded the build.properties from the batik root directory.

> Finally, I believe the patch should also address some documentation regarding
> the newly introduced features.

Sure. I finally found the time to write a few words about this.
Comment 4 Helder Magalhães 2009-04-26 16:22:30 UTC
(In reply to comment #3)
> Patch version 3

Nice to the patch improving. :-)

From a quick look, without involving any testing, I only noticed a few tab/space mix-up in indenting. As far as I know, Batik only uses spaces, so please consider updating the patch. ;-)
Comment 5 Martin von Gagern 2009-04-28 07:13:09 UTC
Created attachment 23558 [details]
Patch version 4

(In reply to comment #4)
> From a quick look, without involving any testing, I only noticed a few
> tab/space mix-up in indenting. As far as I know, Batik only uses spaces, so
> please consider updating the patch. ;-)

Considered and done.
Comment 6 Helder Magalhães 2009-04-30 14:15:25 UTC
(In reply to comment #5)
> Created an attachment (id=23558) [details]
> Patch version 4

Seems like ready for (a deeper) review now. :-)  Can a Batik committer evaluate this and/or (potentially) delineate a list of actions/tests yet to be addressed?
Comment 7 Martin von Gagern 2009-10-12 05:20:14 UTC
Ping? Please include this patch, or comment on how it needs to be improved.