Janne,
To be honest I hadn't had time to look.
When I got your inquiry, I was inclined to toss it off as "I'm just too
damn busy". BUT, I would like it "fixed" to my satisfaction, and
well... it *is* the holidays here, so I figured I'd try to give myself a
present (its been a while since I worked in JSPWiki).
----
I'll write this email while I progress and edit it afterward.
My notes as follows:
1) I pulled the current 2.8 branch figuring it would be best to work
against something stable.
2) Built in Eclipse.
3) Attempted to run AllTests, vaguely pissed that I had to hack a
properties file myself. Where does it tell a newbie that this is
needed, what is needed in it, etc. What a mess, they should just run.
Hacked at tests\etc\jspwiki.properties.tmpl (but I digress).
4) Ran AllTests, Runs: 942/942 Errors:93 Failures:17
** Some of this is probably because my test environment isn't fully
configured?
** Maybe some of this is because I've got JDK6 on this box?
5) Started spelunking, think I found a good cut-point.
So: it looks like MarkupParser.cleanLink has *lots* of callers funneling
thru it... And I think it's the source of my problems; it's not
__commutative__ ( "Test This", and "TestThis" don't come out the same
way, but "Test This ", and "test This" all come out the same as "Test
This"; <shudder/> even worse: "test this" comes out as "Test this" which
is NOT the same!) ick ack ook. It *cleans* a link name, but doesn't
make any attempt to *normalize* it.
----
It also looks like maybe *some* of the calls into
MarkupParser.wikifyLink should instead be .cleanLink... But I'm not
sure (e.g. look at CommandResolver.getFinalPageName,
JSONSearch.getSuggestions, why not .cleanLink?) My guess is the
expanded chars allowed by cleanLink are not always acceptable to callers
of wikifyLink, this suggests to me the method names could be more clear.
But I digress.
----
My current thinking is that a little hack to the .cleanLink() will do
what I want. TextUtil.beautifyString could be employed in .cleanLink to
make it commutative...
Let me see, how about some test cases to document the desired/expected
behavior?
Okay, test case created and attached to this email. A replacement
.cleanLink is at the end of the test case.
Splice in the replacement .cleanLink() and rerun the unit tests again.
6) Re-Ran AllTests, Runs: 942/942 Errors:99 Failures:99
Hmm, so there are more errors now... Casual investigation makes it
appear that they are "expectation" type errors...
E.g. The MarkupParserTest patch at end shows what I mean...
----
<sigh/> Nothing's ever easy.
Can someone look this over the attached test cases and verify that the
expectations for the "cleaned & normalized" links is okay by everyone?
If so I can hunt down the other Junit errors and create patches for
them. I strongly suggest reviewing the CleanLinkTest.java attached and
thinking about how each case *should* get cleaned. If you have other
test cases to add, do so.
I'll have to look more and think thru just how much of this breakage is
serious. I think that there is another piece of the puzzle (maybe at
the PageManager level) that needs fixed up.
Anyway this test case demonstrates what I think the desired
"normalization" should be like.
Comments more than welcome.
Regards,
John Volkar
PS: I would also like to "fix" the ugly %20 stuff in the URLs, maybe via
a behavior property in DefaultUrlConstructor (or maybe just an alternate
NormalizedShortViewUrlConstructor... For Christmas maybe?
----
Index:
C:/_volkar/workspace/JSPWiki-2.8/tests/com/ecyrd/jspwiki/parser/MarkupPa
rserTest.java
===================================================================
---
C:/_volkar/workspace/JSPWiki-2.8/tests/com/ecyrd/jspwiki/parser/MarkupPa
rserTest.java (revision 728670)
+++
C:/_volkar/workspace/JSPWiki-2.8/tests/com/ecyrd/jspwiki/parser/MarkupPa
rserTest.java (working copy)
@@ -24,17 +24,17 @@
public void testCleanLink1()
{
- assertEquals( "--CleanLink--",
MarkupParser.cleanLink("--CleanLink--") );
+ assertEquals( "--Clean Link--",
MarkupParser.cleanLink("--CleanLink--") );
}
public void testCleanLink2()
{
- assertEquals( "CleanLink",
MarkupParser.cleanLink("??CleanLink??") );
+ assertEquals( "Clean Link",
MarkupParser.cleanLink("??CleanLink??") );
}
public void testCleanLink3()
{
- assertEquals( "Clean (link)", MarkupParser.cleanLink("Clean
(link)") );
+ assertEquals( "Clean (Link)", MarkupParser.cleanLink("Clean
(link)") );
}
public static Test suite()
