On Monday 25 March 2002 06:36 am, Erik Hatcher wrote:
...
> CDATA
...
i appear to be the only one who'll use this ability, so i'll just keep
re-patching my local copy for this capability. i FAR prefer it over the
<commandline> approach, for reasons of brevity.
> You have to 'if' statements with identical conditions. Can't you collapse
> this some?
Those are to implement legacy behaviour correctly while also making
command='' optional if another form of command is set. Note how setCommand()
is called in the first one.
> + public String getText() {
> + return this.cdata;
> + }
>
> Why do you need this? Maybe I missed where its referenced, but we
> typically don't have getters. No big deal though.
Because i design for subclassibility, and had problems subclassing the Cvs
task in 1.4.1 because it was missing getters. i had to override setters just
to get at some variables i needed, and found that to be very annoying. Even
if the data members had been protected, i would have provided a getter
because i feel that protected data (as opposed to functions) is evil (as do
the Ant design guidelines ;).
> + public void addConfiguredCommandline( Commandline c ) {
> + this.addConfiguredCommandline( c, false );
> + }
>
> Why exactly do you need addConfiguredCommandline rather than just
> addCommandline?
For the consistent handling of command='' and <commandline> elements using
the same code. It may be doable with just addCommandline(), but this
sollution seems the most extendable to me.
> + /**
> + * Configures and adds the given Commandline.
> + * @param insertAtStart If true, c is
> + */
> + public void addConfiguredCommandline( Commandline c, boolean
> insertAtStart ) {
>
> Why is this public? I'd recommend private or protected.
Fair enough. i would suggest protected, but that's because i like subclasses
;).
> Index: src/main/org/apache/tools/ant/types/Commandline.java
> ===================================================================
> I'm uncomfortable with the changes made to Commandline, simply because its
> used by other code and we need to be careful these changes don't break
> anything.
It's backwards compatible.
> Why not just set the arguments in the order you want them
> rather than having a positional insert option?
Because that requires duplicate code in AbstractCvsTask because of the way
command='' and <commandline> work.
> I would have to spend a
> fair bit of time analyzing it in more detail to make sure I didn't spot any
> issues - I just don't have that kind of time right now. Anyway you keep
> your changes to just the CVS task code and not touch this module? And you
> did mention breaking Javac while you were at it - making my uneasiness
> factor greater!
But i fixed it, and that's why i'm confident that it doesn't break existing
code. ;) Had it NOT broken anything i would be nervous, but after recovering
from having broken <javac>, i'm not converned.
Thanks for the comments!
----- stephan
Generic Unix Computer Guy
[EMAIL PROTECTED] - http://www.einsurance.de
Office: +49 (89) �552 92 862 Handy: �+49 (179) 211 97 67
"...control is a degree of inhibition, and a system which is perfectly
inhibited is completely frozen." -- Alan W. Watts
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
