>
>
>OK, I have some thoughts on this patch. Firstly, let me say that I think a
>description element is a good thing. In fact I thought that was how the
>target descriptions should have been done (i.e. elements not attributes). My
>concern is the fact that description has been implemented as a data type. It
>IS convenient to implement it that way, but conceptually it feels wrong. The
>description, is not a datatype. The fact that this datatype does not hold
>its own data but redirects it straight into the project object is an
>indicator of this mismatch, IMHO.
>
Actually, I have to admit I agree with your conceptual taste here. Back
in June I originally proposed to
emulate Property (
http://www.mail-archive.com/ant-dev%40jakarta.apache.org/msg07491.html )
because you eventually want project-wide descriptions (analogous to
global properties) _and_ target descriptions
(analogous to target-specific properties). I was more of a
ant-coder-newbie at that time and was concerned
about the magnitude of such a change. Stefan suggested implementing it
as a datatype, and that's what I did.
Stefan, please correct me if I am wrong, but I assumed your reasons for
suggesting the DataType
implementation were that you liked the functionality and saw the
DataType approach as an easy,
low-cost, low-risk way to get it into the build. We could always change
it and enhance later....?
I would be happy to submit a revised version of <description> which
could provide something
like the following enhancements:
- multiple instances of <description> at the same level are concatentated
- works at project level and/or target level
- reimplemented as a task along lines of Property
- maintain target level description attributes for backward compatibility
Note that, while this is a fair chunk of coding work, it maintains
buildfile backward compatibility.
>BTW, You can't currently have two <description> elements although I guess we
>could decide what that means and potentially concatenate them.
>
Yes. If that is what we want, maybe its better to make that (trivial)
change now so there is
less of a backward compatibility problem. One way to do this is to change:
addText(String text) {
project.setDescription(text);
}
to something like this:
addText(String text) {
StringBuffer sb = new StringBuffer();
if (project.getDescription()) {
sb.append(project.getDescription());
sb.append(System.getProperty("line.separator"));
}
sb.append(text);
project.setDescription(sb.toString());
}
in types/Description.java Thoughts?
>I would have thought this element should have been handled directly in the
>ProjectHelper parsing code.
>
You mean analogous to the way Property is handled, right? (see above)
>It also creates little oddities such as
>
><project name="test" default="main">
> <description>All your buildfile are belong to us</description>
> <target name="main">
> <description>The main event</description>
> </target>
>
> <target name="never">
> <description>No GST, never ever</description>
> </target>
></project>
>
....where the output of --projecthelp is "No GST, never ever"
That is correct. This potential ugliness can be documented, however.
Question to committers: what course of action do you recommend? Is
there anything here
serious enough to merit a beta-bug-fix?
>Conor
>
--Craeg
|