Wikkid Wiki

Merge lp://staging/~hideaki-t/wikkid/unicode_test into lp://staging/wikkid

unicode_test
Merge into trunk

Proposed by Hideaki Takahashi on 2011-05-14

Status:	Needs review
Proposed branch:	lp://staging/~hideaki-t/wikkid/unicode_test
Merge into:	lp://staging/wikkid
Diff against target:	146 lines (+17/-13) 7 files modified wikkid/app.py (+1/-1) wikkid/dispatcher.py (+2/-2) wikkid/filestore/bzr.py (+8/-6) wikkid/formatter/creoleformatter.py (+1/-0) wikkid/skin/loader.py (+1/-1) wikkid/view/textfile.py (+3/-2) wikkid/view/urls.py (+1/-1)
To merge this branch:	bzr merge lp://staging/~hideaki-t/wikkid/unicode_test
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Aaron Bentley (community)		Needs Fixing on 2011-06-30
Wikkid Hackers	2011-05-14	Pending
Review via email: mp+60980@code.staging.launchpad.net

Description of the change

I developed non-ASCII character support(path/URI, contents) for wikkid.
I think this contains the solution for Bug #614286(without creole parser changes)

Revision history for this message

Tim Penhey (thumper) wrote on 2011-05-15:

On Sat, 14 May 2011 16:53:24 you wrote:
> Hideaki Takahashi has proposed merging lp:~hideaki-t/wikkid/unicode_test
> into lp:wikkid.
>
> Requested reviews:
> Wikkid Hackers (wikkid)
>
> For more details, see:
> https://code.launchpad.net/~hideaki-t/wikkid/unicode_test/+merge/60980
>
> I developed non-ASCII character support(path/URI, contents) for wikkid.
> I think this contains the solution for Bug #614286(without creole parser
> changes)

Thanks for that. the changes generally look good.

Do you have any suggestions for testing it?

Tim

Revision history for this message

Hideaki Takahashi (hideaki-t) wrote on 2011-05-17:

Hello,
I'm sorry my delay.

Thank you for your review.

Ideally all operations should be tested.

I think the following tests will be performed at least.
* file/uri operations(non-ASCII)
  * create new file
  * open(view)
  * edit & save
  * display the file name in a list view
* contentes rendering
  * rendering the contents that contains non-ASCII characters.

Actually I have not unittested that yet. It was regression tested by "make check". and I tested it for the Japanese characters handling using web browser manually.

I propose to create(or add) Japanese version of existing tests. What do you think?
(maybe it is ugly for non japanese speakers if I add Japanese version to existing tests.)

And, I think it should be tested by Latin characters. I just tested it for Japanese characters.

Hideaki

Revision history for this message

Aaron Bentley (abentley) wrote on 2011-06-30:

This doesn't look correct, because the WorkingTree API accepts unicode strings (e.g. in path2id), not bytes or escaped values.

review: Needs Fixing

Revision history for this message

Hideaki Takahashi (hideaki-t) wrote on 2011-07-02:

Thank you for pointing that out. I did not realize that specs.
I'll check and fix the issue.

Hideaki