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

Proposed by Hideaki Takahashi on 2011-05-14
Status: Needs review
Proposed branch: lp:~hideaki-t/wikkid/unicode_test
Merge into: lp: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:~hideaki-t/wikkid/unicode_test
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Needs Fixing on 2011-06-30
Wikkid Hackers 2011-05-14 Pending
Review via email: mp+60980@code.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)

To post a comment you must log in.
Tim Penhey (thumper) wrote :

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

Hideaki Takahashi (hideaki-t) wrote :

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

Aaron Bentley (abentley) wrote :

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

review: Needs Fixing
Hideaki Takahashi (hideaki-t) wrote :

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

Hideaki

Unmerged revisions

61. By Hideaki Takahashi on 2011-05-14

revert #50/#51.

pass unicode string to the creole parser.

60. By Hideaki Takahashi on 2011-05-14

support non-ascii contents in preview.

59. By Hideaki Takahashi on 2011-05-14

merge trunk

58. By Hideaki Takahashi on 2011-02-18

merge trunk changes.

57. By Hideaki Takahashi on 2010-10-28

merge trunk changes

56. By Hideaki Takahashi on 2010-10-16

Convert content to utf-8.
Use safe_utf8 function provided by bzrlib to convert to utf-8.

55. By Hideaki Takahashi on 2010-10-15

don't decode string object.

54. By Hideaki Takahashi on 2010-10-15

Support MS Windows environment.
Use os module insted of bzrlib.urlutils for non bzr controled contents.

53. By Hideaki Takahashi on 2010-10-10

At first, convert the path string to unicode.

52. By Hideaki Takahashi on 2010-10-10

support non ascii filename/URI

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'wikkid/app.py'
2--- wikkid/app.py 2010-11-22 00:00:31 +0000
3+++ wikkid/app.py 2011-05-14 04:53:22 +0000
4@@ -64,7 +64,7 @@
5 request = Request(environ)
6
7 # TODO: reject requests that aren't GET or POST
8- path = urllib.unquote(request.path)
9+ path = urllib.unquote(request.path).decode('utf-8')
10 if path == '/favicon.ico':
11 if self.skin.favicon is not None:
12 response = serve_file(self.skin.favicon)
13
14=== modified file 'wikkid/dispatcher.py'
15--- wikkid/dispatcher.py 2010-11-12 09:00:42 +0000
16+++ wikkid/dispatcher.py 2011-05-14 04:53:22 +0000
17@@ -13,7 +13,7 @@
18
19 import os
20
21-from bzrlib.urlutils import dirname, joinpath
22+from bzrlib.urlutils import joinpath
23
24 from zope.interface import providedBy
25
26@@ -69,7 +69,7 @@
27 # themselves with the view registry.
28
29 def load_view_modules():
30- curr_dir = os.path.abspath(dirname(__file__))
31+ curr_dir = os.path.abspath(os.path.dirname(__file__))
32 view_dir = joinpath(curr_dir, 'view')
33 py_files = [
34 filename for filename in os.listdir(view_dir)
35
36=== modified file 'wikkid/filestore/bzr.py'
37--- wikkid/filestore/bzr.py 2010-10-17 01:47:00 +0000
38+++ wikkid/filestore/bzr.py 2011-05-14 04:53:22 +0000
39@@ -13,10 +13,10 @@
40
41 from bzrlib.errors import BinaryFile
42 from bzrlib.merge3 import Merge3
43-from bzrlib.osutils import split_lines
44+from bzrlib.osutils import split_lines, safe_utf8
45 from bzrlib.revision import NULL_REVISION
46 from bzrlib.textfile import check_text_path
47-from bzrlib.urlutils import basename, dirname, joinpath
48+from bzrlib.urlutils import basename, dirname, joinpath, escape
49
50 from wikkid.errors import FileExists, UpdateConflicts
51 from wikkid.filestore.basefile import BaseFile
52@@ -66,6 +66,9 @@
53 commit_message = 'No description of change given.'
54 if parent_revision is None:
55 parent_revision = NULL_REVISION
56+ # bzr transport expects URL(ASCII only)
57+ path = escape(path)
58+ content = safe_utf8(content)
59 # Firstly we want to lock the tree for writing.
60 self.working_tree.lock_write()
61 try:
62@@ -118,7 +121,6 @@
63 t = t.clone(dirname(path))
64 t.create_prefix()
65 # Put the file there.
66- # TODO: UTF-8 encode text files?
67 t.put_bytes(basename(path), content)
68 self.working_tree.smart_add([t.local_abspath('.')])
69 self.working_tree.commit(
70@@ -153,12 +155,12 @@
71 if len(new_lines) > 0 and not new_lines[-1].endswith(ending):
72 new_lines[-1] += ending
73 merge = Merge3(basis_lines, new_lines, current_lines)
74- result = list(merge.merge_lines()) # or merge_regions or whatever
75+ result = ''.join(merge.merge_lines()) # or merge_regions or whatever
76 conflicted = ('>>>>>>>' + ending) in result
77 if conflicted:
78- raise UpdateConflicts(''.join(result), current_rev)
79+ raise UpdateConflicts(result, current_rev)
80 else:
81- wt.bzrdir.root_transport.put_bytes(path, ''.join(result))
82+ wt.bzrdir.root_transport.put_bytes(path, result)
83 wt.commit(
84 message=commit_message, authors=[author],
85 specific_files=[path])
86
87=== modified file 'wikkid/formatter/creoleformatter.py'
88--- wikkid/formatter/creoleformatter.py 2010-05-12 10:39:13 +0000
89+++ wikkid/formatter/creoleformatter.py 2011-05-14 04:53:22 +0000
90@@ -30,4 +30,5 @@
91
92 def format(self, filename, text):
93 """Format the text."""
94+ text = text.decode('utf-8')
95 return HtmlEmitter(Parser(text, self.rules).parse()).emit()
96
97=== modified file 'wikkid/skin/loader.py'
98--- wikkid/skin/loader.py 2010-11-05 07:43:31 +0000
99+++ wikkid/skin/loader.py 2011-05-14 04:53:22 +0000
100@@ -39,7 +39,7 @@
101 'missing': self.env.get_template('missing-page.html'),
102 'missing-dir' : self.env.get_template('missing-directory.html')
103 }
104- module_location = urlutils.dirname(__file__)
105+ module_location = os.path.dirname(__file__)
106 self.dir_name = urlutils.joinpath(module_location, skin_name)
107
108 def get_template(self, template_name):
109
110=== modified file 'wikkid/view/textfile.py'
111--- wikkid/view/textfile.py 2011-04-23 08:49:27 +0000
112+++ wikkid/view/textfile.py 2011-05-14 04:53:22 +0000
113@@ -7,6 +7,7 @@
114 """View classes to control the rendering of text pages."""
115
116 import logging
117+from urllib import quote
118
119 from webob.exc import HTTPSeeOther
120
121@@ -64,13 +65,13 @@
122 self.content = content
123 default_format = self.execution_context.default_format
124 self.preview_content = format_content(
125- content, self.context.base_name, default_format)
126+ content.encode('utf-8'), self.context.base_name, default_format)
127 else:
128 try:
129 self.context.put_bytes(
130 content, self.user.committer_id, rev_id, description)
131
132- raise HTTPSeeOther(location=self.context.path)
133+ raise HTTPSeeOther(location=quote(self.context.path.encode('utf-8')))
134 except UpdateConflicts, e:
135 # Show the edit page again.
136 logger = logging.getLogger('wikkid')
137
138=== modified file 'wikkid/view/urls.py'
139--- wikkid/view/urls.py 2010-06-16 10:29:35 +0000
140+++ wikkid/view/urls.py 2011-05-14 04:53:22 +0000
141@@ -32,4 +32,4 @@
142 else:
143 if path == '/':
144 path = ''
145- return '{0}/+{1}'.format(path, view)
146+ return u'{0}/+{1}'.format(path, view)

Subscribers

People subscribed via source and target branches