Thanks, feel free to ask any questions you have about any of the changes, since there are a lot of them.  We could also break it into multiple changes if that's more digestible.

I think it's right.  The 'else' of a for-loop will run iff you don't break out of the loop early, which looks like what's intended here.

Wow, I never knew you could do that. I'm not sure what I think of the syntax, but I guess it's valid.. Would you mind putting a comment briefly describing it in the else: just so people don't make the same mistake I did?

I've added a comment in my copy.  It's a feature that's been in Python since the beginning (1.0 at least), and is in the tutorial.

I never read the Tutorial. Guess I should do that sometime :)

Hm, you're right.  I can't think of a good solution to this since p4 output isn't very parsable.  I was thinking a regex like this would work:

 r'(//.+) (//.+) (/.+)'

But that doesn't work in a Windows environment.  Is it safe to assume that this tool requires a unix-like environment?

Also, the older version will choke on spaces, too.  It used this regular expression:

m = re.match(r'%s \/\/.+ (.+)$' % depot_path, where_info)

which doesn't handle spaces (at least in my testing), so there's no regression at least. ;)

We have many Windows users at VMware using post-review.

Our whitespace handling before has been pretty bad and there have been complaints. I'd love to find a good solution for this, but I recognize it's difficult.

According to the p4 where manual (http://www.perforce.com/perforce/doc.current/manuals/cmdref/where.html) we might have multiple results for this command, so maybe we should take this into account.

The goal of this is to get the local path of a file based on the depot path, so maybe we just need to be more specific in our parsing. We can get the local directory the checkout resides in, so perhaps we should just find where that is in each of the resulting lines, and if we find out, grab everything from the beginning of that path to the end of the line.

So if we know the checkout is in /home/bvanzant/home-versioned, we can go through each line and see if that string exists (maybe prepend a space before it so we have a reduced chance of hitting that string in one of the other chunks of that line). Then grab everything from /home/bvanzant/home-versioned through the end of the line, giving us that path. That would take into account whitespace.

I've uploaded a new diff that fixes the issue where "p4 where" might return multiple results.  I think taking the last line is sufficient.  I played around with some workspaces that had multiple mappings and it seems reliable.

As for the whitespace in filenames, I like the idea of looking for the local directory and parsing the output based on that.  I made an experimental version that runs "p4 client -o" to determine the Root and AltRoots.  It's unfortunate to run yet another p4 command, but I can't think of another way.  The --output-diff option seems to work ok, but when I tried to look at the diff posted to my test  server it fails when trying to apply the patch (it says "The patch didn't apply cleanly").  I can post my additional changes to post-review to support spaces in filenames if you'd like.  It's about an additional 30 lines on top of this patch.

Sorry for the delays in getting this and your other patch in. I've been busy with a deadline at work so I haven't really had the time the past couple of weeks to give the patches my full attention, but I'm hoping to do some Review Board work this weekend. At the very least, I'll probably be doing a bit during the week of Thanksgiving.

The patch to support spaces would be nice to get in, but I don't think it needs to be part of this diff. It'd make for an easier review if it's separate anyway.

Just uploaded an update that should apply against the latest svn version.

Looks good.  Committed to SVN.  Thanks!