You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Erik Hatcher <ja...@ehatchersolutions.com> on 2001/12/01 03:43:58 UTC
Re: Immutability improvement patch
----- Original Message -----
From: "Peter Donald" <pe...@apache.org>
> almost. You missed a case. When <antcall/> uses propertys internally it
will
> display a warning when it shouldn't. I have reattached your test file with
a
> small demo and you test it by running test8
>
> Im not sure what the best way to fix this is. What do you think?
Well, you got me there! :)
This one is an interesting one, as it comes close to the discussions about
"privileged" tasks being able to do more to get around things. But maybe
this idea will work:
How about making Project have another constructor that takes a Hashtable of
properties to initialize it with? This would require some refactoring in
Ant.initializeProject() so that it builds a temporary Hashtable of
properties and constructs (although the re-working of things might cause
some issues, not sure) the Project object using the properties constructor
instead of the empty one.
Thoughts? Any drawbacks to another constructor? I haven't looked at
Project's code to see what is involved there, although hopefully/probably
not much.
It doesn't seem like a straight-forward one-liner to allow <ant>/<antcall>
to essentially "override" properties when we don't want Project to allow
this.... but with a new Project constructor Ant.java could do the dirty work
of constructing a list of properties before constructing the Project by
walking through the ones it wants to carry to the new Ant invocation and
setting the embedded override ones in a temporary Hashtable first, giving
precedence to the embedded ones.
> > I'll go back and check all the e-mails on this topic tomorrow to make
sure
> > all the bases have been covered. The one loose end is that
getProperties
> > and getUserProperties return back the actual Hashtable object - so to
close
> > that hole we need to make it return a copy instead. Anyone object to
that
> > copy being made and returned?
>
> +1 to copy ... is there an actual use case for grabbing all properties
> anyways?
How about calls like this:
ProjectHelper.replaceProperties(project, msg, project.getProperties());
(from Echo.java)
although in this case is there a reason why this substitution isn't done
before calling addText? Its done automatically on calls to setXXX methods.
Do we not always want property expansion in element text? I guess <script>
wouldn't be too happy with that, huh?! :)
<junit> uses it to gather the list of properties to put in the output XML
files.
Your favorite: <script> uses it to expose properties to the BSF engine. :)
And it shows up in a few other places (getProperties, that is).
getUserProperties is not used except in Ant, Project, and Script, so its not
a big deal, but it should be cloned also before returning of course.
I didn't check to see if there is code that is relying on changing a
property via the handle from getProperties... I sure hope not!
> BTW could you name your patches something more specific (like
> immutability.patch) - I got so many patchfile.txts it is not funny ;)
No problem. I was just following the rules!
> BTW2 would you consider making immutability.xml part of a Junit testcase ?
;)
Attached. I actually attached it to my original message too! :))
> BTW3 would you mind sending apatch for WHATSNEW to describe these features
Sure... will do that this weekend.
> BTW4 maybe even add a FAQ entry because I am sure the new warnings are
bound
> to cause a few quesions ;)
Not a problem.... on my to-do list with the WHATSNEW and a few other
remaining things related to this. The <antcall> issue is still unresolved -
hopefully some discussion on that will follow so we can figure out the best
way to handle it.
Anyone want to remove Project.unsetProperty? Or shall I submit a patch for
that removal too? :) Thanks Stefan for refactoring your test case for
this.
Property immutability.... perhaps it will now be a concrete reality! :))
Erik
Re: Immutability improvement patch
Posted by Peter Donald <pe...@apache.org>.
On Sat, 1 Dec 2001 15:41, Magesh Umasankar wrote:
> From: "Peter Donald" <pe...@apache.org>
>
> > > > BTW could you name your patches something more specific (like
> > > > immutability.patch) - I got so many patchfile.txts it is not funny ;)
> > >
> > > No problem. I was just following the rules!
> >
> > what rules are these? Is it written somewhere? If so I am going to
>
> exercise
>
> > my HTML hacking skills ;)
>
> http://jakarta.apache.org/site/bugs.html
fixed.
--
Cheers,
Pete
"The perfect way is only difficult for those who pick and choose. Do not
like, do not dislike; all will then be clear. Make a hairbreadth
difference and heaven and earth are set apart; if you want the truth to
stand clear before you, never be for or against." - Bruce Lee
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Erik Hatcher <ja...@ehatchersolutions.com>.
----- Original Message -----
From: "Jose Alberto Fernandez" <j_...@yahoo.com>
> Is it the main problem the fact that the code uses Property objects to do
the job
> or the fact that you cannot know the "user-properties" before you define
> the new project? Or is it something else?
Yes, the main issue is that it uses Property, which calls
setNewProperty/setUserProperty on its associated Project. So to keep things
gradually going in the right direction, have a look at the patch I'll submit
momentarily. I agree its a hack, but the property immutability issue has
gotten much better.
Just because a core Ant task abuses its privileges some doesn't mean we need
to worry too much about folks copying code since they'll be doing so at
their own risk. I'll put comments warning of it. The committers shouldn't
accept future changes that do this kind of thing, and as long as we make
small steps in the right direction each day we'll get there! :)
> If what you need is the Property objects, one thing we could do is to
subclass
> them in an inner class of Project. (The subclass will be priveledge in the
sense
> that it would be able to set the property as a user property.
Not a bad idea. I'll finish off what I'm working on and your way could be
another step in the right direction.
> Am I even in the general ballpark?
Yup!
Erik
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Jose Alberto Fernandez <j_...@yahoo.com>.
Erik,
Is it the main problem the fact that the code uses Property objects to do the job
or the fact that you cannot know the "user-properties" before you define
the new project? Or is it something else?
If what you need is the Property objects, one thing we could do is to subclass
them in an inner class of Project. (The subclass will be priveledge in the sense
that it would be able to set the property as a user property.
The regular Property will have setUserProperty as deprecated. So something like:
public class Project .... {
private static class UserProperty extends Property {
UserProperty() { userProperty = true; }
}
}
Now within the code of the Project or some well defined segment,
you can define UserProperties which do the job as expected,
while in the outside world they are still not needed.
This is simillar to your idea of the new constructor, but with the
difference that general tasks cannot abuse it. And yes, I know the
method will just be deprecated (and hence still available),
but I guess it should go thru the deprecation lifecycle of
eventually requiring -legacy for it to work.
Am I even in the general ballpark?
Jose Alberto
From: "Erik Hatcher" <ja...@ehatchersolutions.com>
> ----- Original Message -----
> From: "Jose Alberto Fernandez" <j_...@yahoo.com>
>
> >> How do we get a Project initialized with user properties without going
> >> through a deprecated API?
> >>
> >> Project.setUserProperty is still the same.
> >>
> >> Here are some thoughts on how to proceed:
> >>
> >> - Un-deprecate the piece in Property.addProperty where it calls
> >> setUserProperty.
>
> > -1
>
> But its really the only easy way to accomplish allowing ant/antcall to
> function as they used to without getting deprecation warnings for normal
> activity... at least that I see.
>
> And we just deprecated it. With setUserProperty removed from Property (or
> deprecated), the only way a user property could be set using Property is
> through ant/antcall. Sure its a hack, but no moreso than other things under
> the hood.
>
>
> >> - Keep setUserProperty deprecated (perhaps even remove it since only
> >> <ant>/<antcall> relied on it, and it was never documented).
> >>
>
> > Humm, what do we do with people that cut and paste from <ant> code :-(
>
> cutting and pasting from an undocumented API sounds risky at best. And if
> they were, then they could still get through the back-door by using the new
> Property constructor like I'm proposing.
>
> >> - Add a constructor: Property(boolean userProperty) - this would get
> called
> >> from Ant.createProperty. - this would allow Property to set user
> properties,
> >> but only if the boolean argument constructor argument was called with
> >> "true" - not something that could be done with a build file <property>.
>
> > -1, this would allow developers to write tasks that modify properties
> > which is exactly what we want to avoid.
>
> We're leaving in Project.setProperty and Project.setUserProperty, so we
> can't stop them from modifying properties.
>
> > If I recall properly, we need to be able to construct a list of properties
> > based on the merge of any user properties in the calling Project and
> > any new properties defined in the invocation.
>
> I thought that would be possible until I dug deeper and saw how the Property
> task is being used under the hood. The only other hack I thought of was to
> construct a dummy Project and have Property run against it and pull the
> properties from there and do the overlay that way.... but that seemed too
> involved.
>
> > This may require having a constructor with a signature like:
>
> > public Project(Project caller, Hashtable userProps);
>
> > This constructor will take into account both set of properties.
> > Then the code as today can set any other properties that may be inherited.
>
> > What do you think?
>
> Just won't work with the way ant/antcall use Property - that task is
> associated with the new Project, and calls setNewProperty/setUserProperty on
> it. Only with a dummy Project in the middle to be used to build the
> Hashtable, but that still relies on Property being able to override user
> properties.
>
> Despite the -1's, I'm going to submit my changes as a patch and let everyone
> have a look. They are much less painful than Jose makes it out to be, and
> should do the trick.
>
> Erik
>
>
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Erik Hatcher <ja...@ehatchersolutions.com>.
----- Original Message -----
From: "Jose Alberto Fernandez" <j_...@yahoo.com>
>> How do we get a Project initialized with user properties without going
>> through a deprecated API?
>>
>> Project.setUserProperty is still the same.
>>
>> Here are some thoughts on how to proceed:
>>
>> - Un-deprecate the piece in Property.addProperty where it calls
>> setUserProperty.
> -1
But its really the only easy way to accomplish allowing ant/antcall to
function as they used to without getting deprecation warnings for normal
activity... at least that I see.
And we just deprecated it. With setUserProperty removed from Property (or
deprecated), the only way a user property could be set using Property is
through ant/antcall. Sure its a hack, but no moreso than other things under
the hood.
>> - Keep setUserProperty deprecated (perhaps even remove it since only
>> <ant>/<antcall> relied on it, and it was never documented).
>>
> Humm, what do we do with people that cut and paste from <ant> code :-(
cutting and pasting from an undocumented API sounds risky at best. And if
they were, then they could still get through the back-door by using the new
Property constructor like I'm proposing.
>> - Add a constructor: Property(boolean userProperty) - this would get
called
>> from Ant.createProperty. - this would allow Property to set user
properties,
>> but only if the boolean argument constructor argument was called with
>> "true" - not something that could be done with a build file <property>.
> -1, this would allow developers to write tasks that modify properties
> which is exactly what we want to avoid.
We're leaving in Project.setProperty and Project.setUserProperty, so we
can't stop them from modifying properties.
> If I recall properly, we need to be able to construct a list of properties
> based on the merge of any user properties in the calling Project and
> any new properties defined in the invocation.
I thought that would be possible until I dug deeper and saw how the Property
task is being used under the hood. The only other hack I thought of was to
construct a dummy Project and have Property run against it and pull the
properties from there and do the overlay that way.... but that seemed too
involved.
> This may require having a constructor with a signature like:
> public Project(Project caller, Hashtable userProps);
> This constructor will take into account both set of properties.
> Then the code as today can set any other properties that may be inherited.
> What do you think?
Just won't work with the way ant/antcall use Property - that task is
associated with the new Project, and calls setNewProperty/setUserProperty on
it. Only with a dummy Project in the middle to be used to build the
Hashtable, but that still relies on Property being able to override user
properties.
Despite the -1's, I'm going to submit my changes as a patch and let everyone
have a look. They are much less painful than Jose makes it out to be, and
should do the trick.
Erik
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Jose Alberto Fernandez <j_...@yahoo.com>.
From: "Erik Hatcher" <ja...@ehatchersolutions.com>
> Actually this turns out to be a bit of a mess. I started digging into
> making these changes. Let me try to summarize what I've found:
>
> - <ant>/<antcall> use the Property task internally to set properties to the
> the new Project instance.
>
> - To set the properties as user properties, they take advantage of
> Project.setUserProperty - which I deprecated in my patch.
>
> - We still need the rich features that the Property task gives us within the
> context of <ant>/<antcall>, and we still need to be able to define those
> properties as user properties on the new Project instance.
>
I agree the functionality needs to be maintained.
> How do we get a Project initialized with user properties without going
> through a deprecated API?
>
> Project.setUserProperty is still the same.
>
> Here are some thoughts on how to proceed:
>
> - Un-deprecate the piece in Property.addProperty where it calls
> setUserProperty.
>
-1
> - Keep setUserProperty deprecated (perhaps even remove it since only
> <ant>/<antcall> relied on it, and it was never documented).
>
Humm, what do we do with people that cut and paste from <ant> code :-(
> - Add a constructor: Property(boolean userProperty) - this would get called
> from Ant.createProperty. - this would allow Property to set user properties,
> but only if the boolean argument constructor argument was called with
> "true" - not something that could be done with a build file <property>.
>
-1, this would allow developers to write tasks that modify properties
which is exactly what we want to avoid.
> It seems that I stumbled upon some other roadblocks in this, but I can't
> recall them at the moment. Let me know if you think the above modifications
> will do the trick.
>
If I recall properly, we need to be able to construct a list of properties
based on the merge of any user properties in the calling Project and
any new properties defined in the invocation.
This may require having a constructor with a signature like:
public Project(Project caller, Hashtable userProps);
This constructor will take into account both set of properties.
Then the code as today can set any other properties that may be inherited.
What do you think?
Jose Alberto
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Erik Hatcher <ja...@ehatchersolutions.com>.
----- Original Message -----
From: "Peter Donald" <pe...@apache.org>
> > How about making Project have another constructor that takes a Hashtable
of
> > properties to initialize it with? This would require some refactoring
in
> > Ant.initializeProject() so that it builds a temporary Hashtable of
> > properties and constructs (although the re-working of things might cause
> > some issues, not sure) the Project object using the properties
constructor
> > instead of the empty one.
>
> that sounds good to me ... but then again I am not the most familiar with
> this - Stefan ? Conor ?
Actually this turns out to be a bit of a mess. I started digging into
making these changes. Let me try to summarize what I've found:
- <ant>/<antcall> use the Property task internally to set properties to the
the new Project instance.
- To set the properties as user properties, they take advantage of
Project.setUserProperty - which I deprecated in my patch.
- We still need the rich features that the Property task gives us within the
context of <ant>/<antcall>, and we still need to be able to define those
properties as user properties on the new Project instance.
How do we get a Project initialized with user properties without going
through a deprecated API?
Project.setUserProperty is still the same.
Here are some thoughts on how to proceed:
- Un-deprecate the piece in Property.addProperty where it calls
setUserProperty.
- Keep setUserProperty deprecated (perhaps even remove it since only
<ant>/<antcall> relied on it, and it was never documented).
- Add a constructor: Property(boolean userProperty) - this would get called
from Ant.createProperty. - this would allow Property to set user properties,
but only if the boolean argument constructor argument was called with
"true" - not something that could be done with a build file <property>.
It seems that I stumbled upon some other roadblocks in this, but I can't
recall them at the moment. Let me know if you think the above modifications
will do the trick.
Thanks,
Erik
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Erik Hatcher <ja...@ehatchersolutions.com>.
----- Original Message -----
From: "Peter Donald" <pe...@apache.org>
> > <junit> uses it to gather the list of properties to put in the output
XML
> > files.
>
> Someone wanna pretend that I am stupid and explain to me in simple terms
why
> this is wanted/desired and what it is used for ?
Well, it had been requested by folks before ("What O/S did my tests run
on?", "When were the tests run?", "What JVM?", "What was the classpath?",
"What was ${my.custom.prop} set to?").
Then some crazy developer learning Ant's internals did this article:
http://www-106.ibm.com/developerworks/java/library/j-junitmail/
and added it. Its got its share of drawbacks - the files get bigger, for
instance.
> > > BTW could you name your patches something more specific (like
> > > immutability.patch) - I got so many patchfile.txts it is not funny ;)
> >
> > No problem. I was just following the rules!
>
> what rules are these? Is it written somewhere? If so I am going to
exercise
> my HTML hacking skills ;)
I went to look at the link Magesh sent, and thought I was losing my mind,
then saw your "fixed" message! :)
Erik
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Magesh Umasankar <um...@apache.org>.
From: "Peter Donald" <pe...@apache.org>
> > > BTW could you name your patches something more specific (like
> > > immutability.patch) - I got so many patchfile.txts it is not funny ;)
> >
> > No problem. I was just following the rules!
>
> what rules are these? Is it written somewhere? If so I am going to
exercise
> my HTML hacking skills ;)
http://jakarta.apache.org/site/bugs.html
Magesh
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Peter Donald <pe...@apache.org>.
On Sat, 1 Dec 2001 13:43, Erik Hatcher wrote:
> Well, you got me there! :)
;)
> How about making Project have another constructor that takes a Hashtable of
> properties to initialize it with? This would require some refactoring in
> Ant.initializeProject() so that it builds a temporary Hashtable of
> properties and constructs (although the re-working of things might cause
> some issues, not sure) the Project object using the properties constructor
> instead of the empty one.
that sounds good to me ... but then again I am not the most familiar with
this - Stefan ? Conor ?
> It doesn't seem like a straight-forward one-liner to allow <ant>/<antcall>
> to essentially "override" properties when we don't want Project to allow
> this.... but with a new Project constructor Ant.java could do the dirty
> work of constructing a list of properties before constructing the Project
> by walking through the ones it wants to carry to the new Ant invocation and
> setting the embedded override ones in a temporary Hashtable first, giving
> precedence to the embedded ones.
>
> > > I'll go back and check all the e-mails on this topic tomorrow to make
>
> sure
>
> > > all the bases have been covered. The one loose end is that
>
> getProperties
>
> > > and getUserProperties return back the actual Hashtable object - so to
>
> close
>
> > > that hole we need to make it return a copy instead. Anyone object to
>
> that
>
> > > copy being made and returned?
> >
> > +1 to copy ... is there an actual use case for grabbing all properties
> > anyways?
>
> How about calls like this:
>
> ProjectHelper.replaceProperties(project, msg, project.getProperties());
> (from Echo.java)
we could replace this with
ProjectHelper.replaceProperties(project, msg)
and let ProjectHelper grab properties out of project. Slightly less coupling.
Actually I may do this through all the tasks.
> although in this case is there a reason why this substitution isn't done
> before calling addText? Its done automatically on calls to setXXX methods.
Its an ugly hack from ye-olde days I believe ;)
> Do we not always want property expansion in element text? I guess <script>
> wouldn't be too happy with that, huh?! :)
yep.
> <junit> uses it to gather the list of properties to put in the output XML
> files.
Someone wanna pretend that I am stupid and explain to me in simple terms why
this is wanted/desired and what it is used for ?
> Your favorite: <script> uses it to expose properties to the BSF engine. :)
kool.
> And it shows up in a few other places (getProperties, that is).
> getUserProperties is not used except in Ant, Project, and Script, so its
> not a big deal, but it should be cloned also before returning of course.
okay.
> I didn't check to see if there is code that is relying on changing a
> property via the handle from getProperties... I sure hope not!
me too ;)
> > BTW could you name your patches something more specific (like
> > immutability.patch) - I got so many patchfile.txts it is not funny ;)
>
> No problem. I was just following the rules!
what rules are these? Is it written somewhere? If so I am going to exercise
my HTML hacking skills ;)
> > BTW2 would you consider making immutability.xml part of a Junit testcase
> > ?
>
> ;)
>
> Attached. I actually attached it to my original message too! :))
oh ... guess I must have missed that ;)
> Not a problem.... on my to-do list with the WHATSNEW and a few other
> remaining things related to this. The <antcall> issue is still unresolved
> - hopefully some discussion on that will follow so we can figure out the
> best way to handle it.
kool.
> Anyone want to remove Project.unsetProperty? Or shall I submit a patch for
> that removal too? :) Thanks Stefan for refactoring your test case for
> this.
I will remove it if there is no objections ...
> Property immutability.... perhaps it will now be a concrete reality! :))
woohoo!
--
Cheers,
Pete
-----------------------------------------------
| If you turn on the light quickly enough, |
| you can see what the dark looks like. |
-----------------------------------------------
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Peter Donald <pe...@apache.org>.
On Mon, 3 Dec 2001 20:34, Stefan Bodewig wrote:
> On Mon, 3 Dec 2001, Peter Donald <pe...@apache.org> wrote:
> >> Moving the three arg method to a utility class and make Project use
> >> it instead of using the two arg version Peter committed might be an
> >> option 8-)
> >
> > or maybe adding a 1 arg version to project
>
> This is what I meant by "make Project use it".
ahh ... I seem to be having trouble understanding people recently ;)
--
Cheers,
Pete
----------------------------------
Ancient Go proverb: "Only amateurs
try to come up with 'fancy' moves"
----------------------------------
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Jose Alberto Fernandez <j_...@yahoo.com>.
From: "Peter Donald" <pe...@apache.org>
> On Mon, 3 Dec 2001 20:10, Stefan Bodewig wrote:
> > On Sat, 1 Dec 2001, Jose Alberto Fernandez <j_...@yahoo.com>
> >
> > wrote:
> > > Saw your changes, do you really think it is necessary to require
> > > people to use ProjectHelper?
> >
> > This is what the current code does and Property relies on having
> > access to the three arg method (in resolveAllProperties). Moving the
> > three arg method to a utility class and make Project use it instead of
> > using the two arg version Peter committed might be an option 8-)
>
> or maybe adding a 1 arg version to project (that in turn delegates to
> ProjectHelper or whatever) which is probably the "right" thing to do.
>
This is exactly what I proposed in my previous message.
Plus, requesting the old methods to be deprecated.
Jose Alberto
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Stefan Bodewig <bo...@apache.org>.
On Mon, 3 Dec 2001, Peter Donald <pe...@apache.org> wrote:
>> Moving the three arg method to a utility class and make Project use
>> it instead of using the two arg version Peter committed might be an
>> option 8-)
>
> or maybe adding a 1 arg version to project
This is what I meant by "make Project use it".
Stefan
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Peter Donald <pe...@apache.org>.
On Mon, 3 Dec 2001 20:10, Stefan Bodewig wrote:
> On Sat, 1 Dec 2001, Jose Alberto Fernandez <j_...@yahoo.com>
>
> wrote:
> > Saw your changes, do you really think it is necessary to require
> > people to use ProjectHelper?
>
> This is what the current code does and Property relies on having
> access to the three arg method (in resolveAllProperties). Moving the
> three arg method to a utility class and make Project use it instead of
> using the two arg version Peter committed might be an option 8-)
or maybe adding a 1 arg version to project (that in turn delegates to
ProjectHelper or whatever) which is probably the "right" thing to do.
--
Cheers,
Pete
---------------------------------
I think not; therefore I ain't...
---------------------------------
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Jose Alberto Fernandez <j_...@yahoo.com>.
From: "Erik Hatcher" <ja...@ehatchersolutions.com>
> > Saw your changes, do you really think it is necessary to require people to
> use
> > ProjectHelper? That class seem to me should be considered an internal API.
> > If it wasn't for deprecation I would suggest make it package scope.
>
> Just to clarify: the above line of code was not part of any or my changes...
> that was already part of Echo.java.
>
> Peter has already addressed this issue somewhat by other commits.
>
Yes I noticed, after I send the message that Peter was the one in the commit messages.
In any case my comments, I think, still applied. :-)
Jose Alberto
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Erik Hatcher <ja...@ehatchersolutions.com>.
----- Original Message -----
From: "Jose Alberto Fernandez" <j_...@yahoo.com>
>> ProjectHelper.replaceProperties(project, msg,
project.getProperties());
>> (from Echo.java)
>>
>> although in this case is there a reason why this substitution isn't done
>> before calling addText? Its done automatically on calls to setXXX
methods.
>> Do we not always want property expansion in element text? I guess
<script>
>> wouldn't be too happy with that, huh?! :)
>>
> Saw your changes, do you really think it is necessary to require people to
use
> ProjectHelper? That class seem to me should be considered an internal API.
> If it wasn't for deprecation I would suggest make it package scope.
Just to clarify: the above line of code was not part of any or my changes...
that was already part of Echo.java.
Peter has already addressed this issue somewhat by other commits.
Erik
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Stefan Bodewig <bo...@apache.org>.
On Sat, 1 Dec 2001, Jose Alberto Fernandez <j_...@yahoo.com>
wrote:
> Saw your changes, do you really think it is necessary to require
> people to use ProjectHelper?
This is what the current code does and Property relies on having
access to the three arg method (in resolveAllProperties). Moving the
three arg method to a utility class and make Project use it instead of
using the two arg version Peter committed might be an option 8-)
Stefan
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Immutability improvement patch
Posted by Jose Alberto Fernandez <j_...@yahoo.com>.
From: "Erik Hatcher" <ja...@ehatchersolutions.com>
> How about calls like this:
>
> ProjectHelper.replaceProperties(project, msg, project.getProperties());
> (from Echo.java)
>
> although in this case is there a reason why this substitution isn't done
> before calling addText? Its done automatically on calls to setXXX methods.
> Do we not always want property expansion in element text? I guess <script>
> wouldn't be too happy with that, huh?! :)
>
Saw your changes, do you really think it is necessary to require people to use
ProjectHelper? That class seem to me should be considered an internal API.
If it wasn't for deprecation I would suggest make it package scope.
What I would suggest is adding a proper entry point to Project:
public String Project.replaceProperties(String s);
which is what it is really needed.
And in case you keep your changes as you have, would you be willing to deprecate:
String ProjectHelper.replaceProperties(Project, String, Hashtable)
now that you added:
public String ProjectHelper.replaceProperties(Project, String)
What I was thinking was renaming the former into something like:
private static String ProjectHelper.replacePropertiesFromHash(Project, String, Hashtable)
which will be called by the 2 arg replaceProperties() and then redefine
the old 3 args as follows (just for deprecation).
/**
* @deprecated Use the new blah blah blah instead
*/
public static String ProjectHelper.replaceProperties(Project p , String s, Hashtable h)
{
return replacePropertiesFromHash(Project p, String s, Hashtable h);
}
The point is that when people compile their own tasks, they get warned they are
using the wrong thing.
If instead, you agree that adding the method to Project is best, then making
replacePropertiesFromHash package wide would probably do, and would
allow for deprecation of the old entry point.
Hope no one thinks these suggestions are controversial,
Jose Alberto
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>