Summary:

Patch: Enable support for paths with spaces in Mercurial (hg) diff parser

Review Request #451 — Created July 16, 2008 and submitted

Information
Owner:	derek
Repository:	Review Board SVN (deprecated)
Branch:
Bugs:	541
Depends On:
Reviewers
Groups:	reviewboard
People:

Description

Enables the Mercurial diff parser to handle file paths with spaces.

I don't know Python, and was therefore "coding by Google", so please expect to find some issues.

Update: diff #2 comes with proposed changes generously applied by bboissin (via http://code.google.com/p/reviewboard/issues/detail?id=541)

Testing Done

Tested locally on Debian with various changesets that do and do not include paths with spaces.

trunk/reviewboard/scmtools/hg.py (Diff revision 1)

Are we guaranteed that diffLine[3] will be "-r" when it's uncommitted?

derek July 23, 2008, 11:50 a.m.

If it's uncommitted, you will get one of:

1. diff -r abcdefghi1234 path/file.name
2. diff -r abcdefghi1234 path/file name with some spaces.name

If it's committed, you will get:

3. diff -r abcdefghi1234 -r ihgfedcba4321 path/file.name
4. diff -r abcdefghi1234 -r ihgfedcba4321 path/file name with some spaces.name

If you read carefully you will find that it is broken when the file name is "-r". "hg add" won't let me add a file with that name, though, so I think it works. There might be some other way to sneak one in, but I don't expect that it's a major concern.

chipx86 July 23, 2008, 5:18 p.m.

Okay. Can you add some comments with those examples to show that above the code?

trunk/reviewboard/scmtools/hg.py (Diff revision 1)

We usually prefer these to be more verbose.

What you could so is:

isCommitted = ...

if isCommitted:
   nameStartIndex = 5
   info[...]
else:
   nameStartIndex = 3
   info[...]

derek July 23, 2008, 11:47 a.m.

Gotcha ... just out of curiosity, does nameStartIndex get scoped to the "try" body or the function?

chipx86 July 23, 2008, 5:18 p.m.
```
The function.
```

trunk/reviewboard/scmtools/hg.py (Diff revision 1)

You should probably use urllib.quote() for escaping contents.

derek July 23, 2008, 11:46 a.m.

Nice ... urlencode is too heavy in this case, didn't realize there was something in between. It even defaults to allowing '/' ... it's almost like this has come up before :)

Ship it!

```
Committed as r1442.  Thanks!!
```