541: Mercurial (hg) scmtools back end does not support file paths with spaces

derek******@gmai***** (Google Code) (Is this you? Claim this profile.)
Aug. 15, 2008 
The Mercurial diff parser currently assumes that files do not have spaces.
This has caused us problems, as we have some files with spaces that need
review.

The attached patch enabled this to work properly in our environment using a
Debian-based server with Python 2.5.2. I will put this on the reviewboard
server shortly.
hg-spaces-in-filename.patch 
Index: scmtools/hg.py
===================================================================
--- scmtools/hg.py	(revision 1401)
+++ scmtools/hg.py	(working copy)
@@ -45,9 +45,12 @@
             diffLine = self.lines[linenum].split()
             try:
                 # hg is file based, so new file always == old file
-                info['newFile'] = info['origFile'] = diffLine[-1]
+                isCommitted = len(diffLine) > 4 and diffLine[3] == '-r'
+                nameStartIndex = 5 if isCommitted else 3
+                info['newFile'] = info['origFile'] = \
+                    ' '.join(diffLine[nameStartIndex:])
                 info['origInfo'] = diffLine[2]
-                if len(diffLine) <= 4:
+                if not isCommitted:
                     info['newInfo'] = "Uncommitted"
                 else:
                     info['newInfo'] = diffLine[4]
@@ -90,7 +93,7 @@
             passman.add_password(None, self.url, self.username, self.password)
             authhandle
#1 derek******@gmai***** (Google Code) (Is this you? Claim this profile.) 
http://reviews.review-board.org/r/451/
#2 bboi****@gmai***** (Google Code) (Is this you? Claim this profile.) 
I needed the exact same thing.

Updated patch below
  • +
    reviewboard.diff 
    Index: reviewboard/scmtools/hg.py
    ===================================================================
    --- reviewboard/scmtools/hg.py	(revision 1414)
    +++ reviewboard/scmtools/hg.py	(working copy)
    @@ -15,7 +15,7 @@
                 self.client = HgClient(repository.path)
     
         def get_file(self, path, revision=HEAD):
    -        return self.client.cat_file(path, str(revision))
    +        return self.client.cat_file(path.encode('utf8'), str(revision))
     
         def parse_diff_revision(self, file_str, revision_str):
             revision = revision_str
    @@ -40,17 +40,25 @@
         """
     
         def parse_special_header(self, linenum, info):
    +        # XXX: does not handle git style diffs
             if self.lines[linenum].startswith("diff -r"):
    -            # should contain "diff -r aaa [-r bbb] filename"
    +            # diff between two revisions are in the following form:
    +            #  "diff -r abcdef123456 -r 123456abcdef filename"
    +            # diff between a revision and the working copy are like:
    +
#3 bboi****@gmai***** (Google Code) (Is this you? Claim this profile.) 
(sorry I don't know how to put the interdiff in review board :/ )
#4 bboi****@gmai***** (Google Code) (Is this you? Claim this profile.) 
And sorry, I messed up. The first hunk is unrelated (it's encoding related).

proper file is attached.
  • +
    reviewboard.diff 
    Index: reviewboard/scmtools/hg.py
    ===================================================================
    --- reviewboard/scmtools/hg.py	(revision 1414)
    +++ reviewboard/scmtools/hg.py	(working copy)
    @@ -40,17 +40,25 @@
         """
     
         def parse_special_header(self, linenum, info):
    +        # XXX: does not handle git style diffs
             if self.lines[linenum].startswith("diff -r"):
    -            # should contain "diff -r aaa [-r bbb] filename"
    +            # diff between two revisions are in the following form:
    +            #  "diff -r abcdef123456 -r 123456abcdef filename"
    +            # diff between a revision and the working copy are like:
    +            #  "diff -r abcdef123456 filename"
                 diffLine = self.lines[linenum].split()
                 try:
                     # hg is file based, so new file always == old file
    -                info['newFile'] = info['origFile'] = diffLine[-1]
    +                isCommitted = len(diffLine) > 4 and diffLine[3] == '-r'
    +                if isCommit
#5 derek******@gmai***** (Google Code) (Is this you? Claim this profile.) 
Thanks bboissin, I am getting destroyed at work right now and wasn't going to get to
that any time soon.

I updated the review request with your changes.
david
#6 david
  • +Fixed