Sorry,
apparently i am being a bit confusing.
the patch was rather large (six lines of code) so i can understand why
i should have explained it more clearly,
The patch is against 3 issues, 1 of them i submitted when the old
bugzilla, the other 2 don't exist. but there is a unit test per each issue
submitted with the patch.
don't quite understand why i would create an issue merely to close it.
i mean if i needed to show my boss i was working the busy work would make
sense, but as we are all doing this
in our free time.... if you feel you need notes that better explain the
patch than the unit tests, by all means fell free, but i personally can't
think what i would write that is more precise than actual code.
issue 1 (this is Velocity-381)
the issue is the is shown by the following test, as in both should
return ""
---
public void testMethod() throws Exception {
assertEquals("", parseString("$!main.toString()", this));
}
public void testField() throws Exception {
assertEquals("", parseString("$!main", this));
}
public String toString() {
return null;
}
----
the line of code change is
- if (value == null)
+ if (value == null || value.toString() == null)
issue 2 (no jira issue)
the info is created wrong.
code change
----
- method = rsvc.getUberspect().getMethod(o, methodName,
params, new Info("",1,1));
+ method = rsvc.getUberspect().getMethod(o, methodName,
params, new Info(context.getCurrentTemplateName(), getLine(),
getColumn()));
-----
the test checks against an uberspect that throws exceptions with it can't
find stuff.
(this is also useful for development, but i am trying to start with small
changes as to keep things simple, so kept it to testing)
---
public Info getInfoFor(String velocity) throws Exception {
try {
parseString(velocity, this);
fail("Uberspect Should have thrown an exception");
throw new Error("Shouldn't be able to reach this point");
} catch (VelocityParsingError t) {
return t.getInfo();
}
}
public void testInfoForField() throws Exception {
Info i = getInfoFor("$main.unknownField");
assertInfoEqual(i, "$main.unknownField", 1, 7);
}
public void testInfoForMethod() throws Exception {
Info i = getInfoFor("$main.unknownMethod()");
assertInfoEqual(i, "$main.unknownMethod()", 1, 7);
}
private void assertInfoEqual(Info i, String name, int line, int column)
{
assertEquals("Template Name", name, i.getTemplateName());
assertEquals("Template Line", line, i.getLine());
assertEquals("Template Column", column, i.getColumn());
}
---
The third test also deals with the uberspect. design indicates that the
uberspect should be asked to find the method, and therefore if you wanted
a to write an uberspect to check when you are calling methods on null's
you could write one. but the parser blocked it.
the test is
---
public void testNullPointer() {
assertErrorThrown("$main.getNull().callMethod()");
}
private void assertErrorThrown(String string) {
try {
String result = parseString(string, this);
Assert.fail("parsing '" + string + "' did not fail but returned '"
+ result + "'");
} catch (Throwable t) {
return;
}
}
----
the code change is the removal of premature exit
- if (result == null)
- {
- return null;
- }
>Really appreciate your contribution - I don't mean to throw up
unnecessary obstacles.
thank you, i look forward to contributing in the future
llewellyn.
----- Original Message -----
From: "Will Glass-Husain" <[EMAIL PROTECTED]>
To: "Velocity Developers List" <velocity-dev@jakarta.apache.org>;
"Llewellyn Falco" <[EMAIL PROTECTED]>
Sent: Thursday, September 22, 2005 11:01 PM
Subject: Re: Patch
Hi Llewllyn,
I'm a bit confused. The patch that you are submitting - does it solve
the
issue in Velocity-381? If so, please attach to that issue.
Does the patch solve a different problem? Then create a new issue in
JIRA,
describe the problem, attach the test, and attach the solution.
I'm eager to review, comment on, (and if they are ready) apply your
patches,
but for ease of processing it's best they go through JIRA. (discussion
can
be on the dev list or on the JIRA issue). Let me know if you've any
difficulties logging on or using the system.
Thanks again,
WILL
----- Original Message -----
From: "Llewellyn Falco" <[EMAIL PROTECTED]>
To: "Velocity Developers List" <velocity-dev@jakarta.apache.org>
Sent: Thursday, September 22, 2005 9:14 PM
Subject: Re: Patch
ok,
you can reproduce all the bug by running the attached tests without
the
patches
(as is the point of unit tests)
The patches fix them, most of them are in the order of 1 line, so
easier to see the code rather than explain.
the only issue i saw that related was.
velocity-381
again each test is pretty straight forward about what it is testing.
llewellyn.
----- Original Message -----
From: "Will Glass-Husain" <[EMAIL PROTECTED]>
To: "Velocity Developers List" <velocity-dev@jakarta.apache.org>;
"Llewellyn Falco" <[EMAIL PROTECTED]>
Sent: Thursday, September 22, 2005 10:29 AM
Subject: Re: Patch
Can you please create a JIRA issue then?
Please list exactly the problem you were experiencing, including how to
reproduce it. Then attach the patch and state how it solves the
problem.
Really appreciate your contribution - I don't mean to throw up
unnecessary obstacles. But it makes a big difference in ease of
tracking
to have every change tracked and submitted through our issue tracker.
Among other benefits, it documents the change for those who experience
the same bug in an earlier version, and it provides a place for other
developers (e.g. me) to ask questions and offer suggestions for
improving
the specific patch. (and the list serv does not always include all
attached files).
Thanks again, WILL
----- Original Message -----
From: "Llewellyn Falco" <[EMAIL PROTECTED]>
To: "Velocity Developers List" <velocity-dev@jakarta.apache.org>
Sent: Thursday, September 22, 2005 10:11 AM
Subject: Re: Patch
I don't know if there are any jira issue's involved.
It solves the issues of i was experiencing of..
null not being able to pass to uberspect to resolve, as is indecated
by
the uberspect implementation and api.
info not being correctly created for a failure in a method call.
if an object is not null, but the toString returns null the silent
failed.
the ant tests passed.
Llewellyn.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
