post-review: Fix handling of spaces in file paths when using Perforce

Review Request #815 — Created April 14, 2009 and submitted

Information

Review Board SVN (deprecated)
840

Reviewers

Fixes a problem where changes files with spaces in their paths would end up appearing as deleted content in Review Board diffs.  Changed the Perforce output format to use the -ztag switch, and then modified the parsing code to pull out the full path including the space(s).

This was a change that I originally attached to the following issue http://code.google.com/p/reviewboard/issues/detail?id=840
Tested on Mac OS X with a changed file that has a space in it's full path.
EH
  1. My recent patch has added a _run_p4 method which uses the marshal API.  You could change this to:
    
    where_output = self._run_p4(['where', depot_path])
    return where_output['path']
    
    This might be a little more reliable than parsing text output.
    
    1. I don't see this in the current distribution of RBTools.
    2. It was checked in about a month ago.  Looks like the last release of RBTools was about 2 months ago.  You can see the code here: http://code.google.com/p/reviewboard/source/browse/trunk/rbtools/scripts/post-review
    3. Yeah, you'll want to develop only against SVN. It's too easy to stay out of sync otherwise.
      
      We'll get to another RBTools release before long. Been concentrating on work and on Review Board itself.
    4. To clarify, it looks to me like _run_p4 was in the version of post-review on the trunk, but not the spaces-in-pathnames fix.  I've now updated this review with a new and improved fix.
  2. 
      
BU
Review request changed

Change Summary:

I finally got around to implementing the change suggested by Eric Huss to use the _run_p4 method.   It seems to work in my light testing.

Diff:

Revision 2 (+4 -12)

Show changes

chipx86
  1. Looks good, thanks. Committed as r1990.
  2. 
      
Loading...