[prev in list] [next in list] [prev in thread] [next in thread]
List: velocity-dev
Subject: Re: Patch
From: "Llewellyn Falco" <isidore () setgame ! com>
Date: 2005-09-23 7:47:01
Message-ID: 004701c5c012$fbbc8eb0$0a01a8c0 () lfalco
[Download RAW message or body]
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" <wglass@forio.com>
To: "Velocity Developers List" <velocity-dev@jakarta.apache.org>; "Llewellyn
Falco" <isidore@setgame.com>
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" <isidore@setgame.com>
> 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" <wglass@forio.com>
>> To: "Velocity Developers List" <velocity-dev@jakarta.apache.org>;
>> "Llewellyn Falco" <isidore@setgame.com>
>> 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" <isidore@setgame.com>
>>> 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: velocity-dev-unsubscribe@jakarta.apache.org
>>>> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>>> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic