5dec221dd171

guts: fix filename encoding

note: this made me drink. a lot.
[view raw] [browse files]
author Steve Losh <steve@stevelosh.com>
date Fri, 09 Jul 2010 21:44:09 -0400
parents 3b303e310e16
children 8ccff9378aba
branches/tags (none)
files review/api.py review/cli.py review/static/comments.js review/static/style.css review/static/style.less review/templates/changeset.html review/templates/pieces/comment.html review/templates/pieces/forms/comment-change.html review/templates/pieces/forms/file-comment.html review/templates/pieces/linecomment.html review/util.py review/web.py

Changes

--- a/review/api.py	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/api.py	Fri Jul 09 21:44:09 2010 -0400
@@ -2,8 +2,9 @@
 
 """The API for interacting with code review data."""
 
-import datetime, operator, os
+import base64, datetime, operator, os
 import messages
+from util import fromlocal
 from mercurial import cmdutil, error, hg, patch, util
 from mercurial.node import hex
 from mercurial import ui as _ui
@@ -148,10 +149,10 @@
     for k, v in json.loads(data).iteritems():
         result[k.encode('UTF-8')] = v
 
-    if u'filename' in result:
-        result['filename'] = result['filename'].encode('UTF-8')
     result['node'] = result['node'].encode('UTF-8')
     result['style'] = result['style'].encode('UTF-8')
+    if 'lines' in result:
+        result['lines'] = map(int, result['lines'])
 
     return result
 
@@ -310,7 +311,8 @@
         else:
             items[0]._delete(self.ui, self.repo)
 
-    def edit_comment(self, identifier, message=None, filename=None, lines=None, style=None):
+    def edit_comment(self, identifier, message=None, ufilename=None, filename=None,
+                     lines=None, style=None):
         olds = self.get_items(identifier)
 
         if len(olds) == 0:
@@ -327,6 +329,7 @@
             raise FileNotInChangeset(filename)
 
         old.hgdate = util.makedate()
+        old.ufilename = ufilename if ufilename is not None else fromlocal(filename)
         old.filename = filename
         old.lines = lines if lines is not None else old.lines
         old.message = message if message is not None else old.message
@@ -374,6 +377,25 @@
     and full_diffs methods.  See the docs of those methods for more info.
 
     """
+    def _load_comment_file(self, filename):
+        data = _parse_data(self.repo['tip'][filename].data())
+
+        data['hgdate'] = util.parsedate(data['hgdate'])
+        data['identifier'] = _split_path_dammit(filename)[-1]
+        data['ufilename'] = data['file'][0]
+        data['filename'] = base64.b64decode(data['file'][1]) if data['file'] else ''
+
+        return ReviewComment(**data)
+
+    def _load_signoff_file(self, filename):
+        data = _parse_data(self.repo['tip'][filename].data())
+
+        data['hgdate'] = util.parsedate(data['hgdate'])
+        data['identifier'] = _split_path_dammit(filename)[-1]
+
+        return ReviewSignoff(**data)
+
+
     def __init__(self, ui, repo, target, node):
         """Initialize a ReviewChangeset.
 
@@ -396,20 +418,10 @@
             commentfns = filter(_match('%s/comments' % node), relevant)
             signofffns = filter(_match('%s/signoffs' % node), relevant)
 
-            self.comments = []
-            for fn in commentfns:
-                data = _parse_data(self.repo['tip'][fn].data())
-                data['hgdate'] = util.parsedate(data['hgdate'])
-                data['identifier'] = _split_path_dammit(fn)[-1]
-                self.comments.append(ReviewComment(**data))
+            self.comments = [self._load_comment_file(fn) for fn in commentfns]
             self.comments.sort(key=operator.attrgetter('local_datetime'))
 
-            self.signoffs = []
-            for fn in signofffns:
-                data = _parse_data(self.repo['tip'][fn].data())
-                data['hgdate'] = util.parsedate(data['hgdate'])
-                data['identifier'] = _split_path_dammit(fn)[-1]
-                self.signoffs.append(ReviewSignoff(**data))
+            self.signoffs = [self._load_signoff_file(fn) for fn in signofffns]
             self.signoffs.sort(key=operator.attrgetter('local_datetime'))
         else:
             self.comments = []
@@ -450,7 +462,7 @@
                                 self.node, opinion, message, style)
         signoff._commit(self.ui, self.repo)
 
-    def add_comment(self, message, filename='', lines=[], style=''):
+    def add_comment(self, message, ufilename=u'', filename='', lines=[], style=''):
         """Add (and commit) a comment for the given file and lines.
 
         The filename should be normalized to the format Mercurial expects,
@@ -466,9 +478,11 @@
         """
         if filename and filename not in self.target[self.node].files():
             raise FileNotInChangeset(filename)
+        if filename and not ufilename:
+            ufilename = fromlocal(filename)
 
         comment = ReviewComment(self.ui.username(), util.makedate(),
-            self.node, filename, lines, message, style)
+            self.node, ufilename, filename, map(int, lines), message, style)
         comment._commit(self.ui, self.repo)
 
 
@@ -660,6 +674,16 @@
         """Return the list of files in the revision for this ReviewChangeset."""
         return self.target[self.node].files()
 
+    def unicode_files(self):
+        """Returns a tuple of files in the revision for this ReviewChangeset.
+        
+        Each element is a pair of strings, the first is a unicode string of the
+        filename, the second is a bytestring of the filename (which Mercurial
+        will want).
+
+        """
+        return [(fromlocal(f), f) for f in self.target[self.node].files()]
+
 
     def review_level_comments(self):
         """Comments on this changeset which aren't on a particular file."""
@@ -775,13 +799,14 @@
     The following pieces of information are stored for comments:
 
         comment = rcset.comments[0]
-        comment.author
+        comment.author                  U
         comment.hgdate
         comment.node
+        comment.ufilename               U
         comment.filename
         comment.lines
         comment.local_datetime
-        comment.message
+        comment.message                 U
         comment.style
         comment.identifier
         comment.itemtype
@@ -797,9 +822,13 @@
     next to the current computer would have read at the instant the comment
     was added.
 
+    ufilename is a unicode string representing the filename.
+
+    filename is a byte string representing the filename.
+
     """
-    def __init__(self, author, hgdate, node, filename, lines, message,
-                 style='', identifier=None, **extra):
+    def __init__(self, author, hgdate, node, ufilename, filename, lines,
+                 message, style='', identifier=None, **extra):
         """Initialize a ReviewComment.
 
         You shouldn't need to create these directly -- use a ReviewChangeset
@@ -818,6 +847,7 @@
         self.author = author
         self.hgdate = hgdate
         self.node = node
+        self.ufilename = ufilename
         self.filename = filename
         self.lines = lines
         self.message = message
@@ -834,7 +864,8 @@
         """
         return json.dumps({ 'author': self.author, 'node': self.node,
                             'hgdate': util.datestr(self.hgdate),
-                            'filename': self.filename, 'lines': self.lines,
+                            'file': [self.ufilename, base64.b64encode(self.filename)],
+                            'lines': self.lines,
                             'style': self.style, 'message': self.message
                           }, indent=4, sort_keys=True)
 
--- a/review/cli.py	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/cli.py	Fri Jul 09 21:44:09 2010 -0400
@@ -6,10 +6,10 @@
 """
 
 import api, helps, messages
+from util import tolocal
 from mercurial import help, templatefilters, util
 from mercurial.node import short
 
-
 def _get_datastore(ui, repo):
     try:
         return api.ReviewDatastore(ui, repo)
@@ -46,7 +46,8 @@
     style = 'markdown' if mdown or comment.style == 'markdown' else ''
 
     try:
-        rd.edit_comment(comment.identifier, message, fnames[0], lines, style)
+        rd.edit_comment(comment.identifier, message,
+                        filename=fnames[0], lines=lines, style=style)
     except api.FileNotInChangeset:
         raise util.Abort(messages.COMMENT_FILE_DOES_NOT_EXIST % (
                              fnames[0], rd.target[comment.node].rev()))
@@ -231,7 +232,7 @@
                  label='review.comment')
 
         for line in comment.message.splitlines():
-            ui.write(messages.REVIEW_LOG_COMMENT_LINE % line, label='review.comment')
+            ui.write(messages.REVIEW_LOG_COMMENT_LINE % tolocal(line), label='review.comment')
 
         ui.write(after)
 
--- a/review/static/comments.js	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/static/comments.js	Fri Jul 09 21:44:09 2010 -0400
@@ -23,7 +23,8 @@
                 '<a class="submit button"><span>Post Comment</span></a>' +
                 '<a class="cancel-line button"><span>Cancel</span></a>' +
 
-                '<input type="hidden" name="filename" value="{{ filename }}" />' +
+                '<input type="hidden" name="filename-u" value="{{ filename_u }}" />' +
+                '<input type="hidden" name="filename-b64" value="{{ filename_b64 }}" />' +
                 '<input type="hidden" name="current" value="{{ identifier }}" class="current" />' +
                 '<input class="lines" type="hidden" name="lines" value="{{ currNum }}" />' +
             '</form>' +
@@ -32,10 +33,12 @@
 );
 
 function RenderLineCommentForm(line, currNum, body, markdown, identifier) {
-    var filename = line.closest(".file").find(".filename h3 a .name").html();
+    var filename_u = line.closest(".file").find(".filename .filename-u").text();
+    var filename_b64 = line.closest(".file").find(".filename .filename-b64").text();
     markdown_checked = markdown ? 'checked="checked"' : '';
-    return comment_form({ filename: filename, currNum: currNum, body: body,
-                          markdown_checked: markdown_checked, identifier: identifier });
+    return comment_form({ filename_u: filename_u, filename_b64: filename_b64,
+                          currNum: currNum, body: body, identifier: identifier,
+                          markdown_checked: markdown_checked });
 }
 
 $(function() {
--- a/review/static/style.css	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/static/style.css	Fri Jul 09 21:44:09 2010 -0400
@@ -395,8 +395,9 @@
 }
 #changeset .content .head .patch {
   position: absolute;
-  top: 43px;
-  right: -16px;
+  top: -18px;
+  right: -80px;
+  width: 50px;
 }
 #changeset .content .head .fulldesc {
   width: 660px;
--- a/review/static/style.less	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/static/style.less	Fri Jul 09 21:44:09 2010 -0400
@@ -383,8 +383,9 @@
         }
         .patch {
             position: absolute;
-            top: 43px;
-            right: -16px;
+            top: -18px;
+            right: -80px;
+            width: 50px;
         }
         .fulldesc {
             width: 660px;
--- a/review/templates/changeset.html	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/templates/changeset.html	Fri Jul 09 21:44:09 2010 -0400
@@ -79,14 +79,15 @@
 
     <h2>Files</h2>
 
-    {% for filename in rcset.files() %}
+    {% for ufilename, filename in rcset.unicode_files() %}
         {% set comments = rcset.file_level_comments(filename) %}
         {% set commentcount = utils['len'](comments) + utils['len'](rcset.line_level_comments(filename)) %}
 
         <div class="file">
             <div class="filename group">
+                <span class="filename-b64 disabled">{{ utils['b64'](filename) }}</span>
                 <h3>
-                    <a class="fold" href="#"><span class="name">{{ filename }}</span>{% if commentcount %} ({{ commentcount }} comment{% if commentcount != 1 %}s{% endif %}){% endif %}</a>
+                    <a class="fold" href="#"><span class="filename-u">{{ ufilename }}</span>{% if commentcount %} ({{ commentcount }} comment{% if commentcount != 1 %}s{% endif %}){% endif %}</a>
                     <span class="status">&rarr;</span>
                 </h3>
             </div>
--- a/review/templates/pieces/comment.html	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/templates/pieces/comment.html	Fri Jul 09 21:44:09 2010 -0400
@@ -36,8 +36,8 @@
                 said:
             </div>
             <div class="context">
-                {% if comment.filename %}
-                    <div class="context-head">on {{ comment.filename }}</div>
+                {% if comment.ufilename %}
+                    <div class="context-head">on {{ comment.ufilename }}</div>
                 {% else %}
                     <div class="context-head">on changeset {{ rev.rev() }}:{{ utils['node_short'](rev.node()) }}</div>
                 {% endif %}
--- a/review/templates/pieces/forms/comment-change.html	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/templates/pieces/forms/comment-change.html	Fri Jul 09 21:44:09 2010 -0400
@@ -22,7 +22,12 @@
             </label>
 
         </div>
-        <input type="hidden" name="filename" value="{{ filename }}" />
+        <input type="hidden" name="filename-u" value="{{ ufilename }}" />
+        {% if filename %}
+            <input type="hidden" name="filename-b64" value="{{ utils['b64'](filename) }}" />
+        {% else %}
+            <input type="hidden" name="filename-b64" value="" />
+        {% endif %}
         <input type="hidden" name="current" value="{{ comment.identifier }}"/>
         <a class="submit button" href="#"><span>Edit Comment</span></a>
         <a class="cancel-edit button" href="#"><span>Cancel</span></a>
--- a/review/templates/pieces/forms/file-comment.html	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/templates/pieces/forms/file-comment.html	Fri Jul 09 21:44:09 2010 -0400
@@ -15,6 +15,7 @@
         <a class="submit button" href="#"><span>Post Comment</span></a>
         <a class="cancel button" href="#"><span>Cancel</span></a>
 
-        <input type="hidden" name="filename" value="{{ filename }}" />
+        <input type="hidden" name="filename-u" value="{{ ufilename }}" />
+        <input type="hidden" name="filename-b64" value="{{ utils['b64'](filename) }}" />
     </form>
 </div>
--- a/review/templates/pieces/linecomment.html	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/templates/pieces/linecomment.html	Fri Jul 09 21:44:09 2010 -0400
@@ -48,7 +48,7 @@
                     said:
                 </div>
                 <div class="context">
-                    <div class="context-head">in {{ comment.filename }}</div>
+                    <div class="context-head">in {{ comment.ufilename }}</div>
                 </div>
                 <div class="message">{{ comment.message }}</div>
             </div>
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/review/util.py	Fri Jul 09 21:44:09 2010 -0400
@@ -0,0 +1,8 @@
+from mercurial import encoding
+
+def tolocal(s):
+    return encoding.tolocal(s.encode('UTF-8'))
+
+def fromlocal(s):
+    return encoding.fromlocal(s).decode('UTF-8')
+
--- a/review/web.py	Tue Jul 06 23:31:10 2010 -0400
+++ b/review/web.py	Fri Jul 09 21:44:09 2010 -0400
@@ -2,8 +2,7 @@
 
 """The review extension's web UI."""
 
-import sys, os
-import StringIO
+import base64, sys, os, StringIO
 from hashlib import md5
 
 from mercurial import cmdutil, commands, hg, templatefilters
@@ -54,6 +53,7 @@
              'no': len(filter(lambda s: s.opinion == 'no', signoffs)),
              'neutral': len(filter(lambda s: s.opinion == '', signoffs)),}
 
+
 markdowner = markdown2.Markdown(safe_mode='escape', extras=['code-friendly', 'pyshell', 'imgless'])
 utils = {
     'node_short': short,
@@ -69,6 +69,7 @@
     'str': str,
     'decode': lambda s: s.decode('utf-8'),
     'markdown': markdowner.convert,
+    'b64': base64.b64encode,
 }
 
 def _render(template, **kwargs):
@@ -127,7 +128,9 @@
     return redirect("%s/changeset/%s/" % (app.site_root, revhash))
 
 def _handle_comment(revhash):
-    filename = request.form.get('filename', '')
+    filename = base64.b64decode(request.form.get('filename-b64', u''))
+    ufilename = request.form.get('filename-u', u'')
+    print repr(filename), repr(ufilename)
 
     lines = str(request.form.get('lines', ''))
     if lines:
@@ -139,10 +142,10 @@
     if body:
         current = request.form.get('current')
         if current:
-            g.datastore.edit_comment(current, body, filename, lines, style)
+            g.datastore.edit_comment(current, body, ufilename, filename, lines, style)
         else:
             rcset = g.datastore[revhash]
-            rcset.add_comment(body, filename, lines, style)
+            rcset.add_comment(body, ufilename, filename, lines, style)
     
     return redirect("%s/changeset/%s/" % (app.site_root, revhash))