Mercurial support

This patch adds support for Mercurial.

It requires this patch:

http://codereview.appspot.com/2891

to be uploaded first.

[Andi Albrecht, 2008-08-13 04:28:47]

[added me to reviewers]

[Antoine Pitrou, 2008-08-13 09:51:46]

Hey,

I don't know if it's a problem on my side (there's a nasty proxy here), but when
I try to view the side-by-side diff (*), I get:

"Bad content. Try to upload again."

(*) the URL is
http://codereview.appspot.com/2878/diff/1/2

cheers

Antoine.

[Antoine Pitrou, 2008-08-13 10:05:32]

Ok, the patch is simple enough to do a review without the side-by-side diff view
:)

A few comments:

+    cmd = "hg diff"

You should use "hg diff --git" explicitly if you expect a git format for the
diff.

+    # bzr diff internally returns True if changes are found (see:
+    # bzrlib.diff.DiffTree._show_diff).

Are you sure those comments still apply? :)

+      m = re.match("diff --git a/(\S+) b/(\S+)", line)

How will it work for filenames with spaces in them?
Are they %20-escaped in this particular line?

+    if self.options.bzr_rev:
+      args += ["-r", self.options.bzr_rev]

s/bzr_rev/hg_rev ?
By the way, what's the point of having two separate options for bzr and hg rev?
You can only use one at a time anyway, so --rev (or -r) would be simpler.

+    for line in status.split("\n"):

Is it ok under Windows (which has "\r\n" endings)? splitlines() seems better to
me.

[frew, 2008-08-14 01:56:18]

+1 on the bad content problem.

This is very cool. Thanks!

+  """Implementation of the VersionControlSystem interface for Bazaar."""

s/Bazaar/Mercurial/

There are a couple of other Bazaar/bzr->Mercurial/hg changes scattered around in
comments.

+       content = RunShell("hg cat", args)

You should pass silent_ok=True to RunShell. Otherwise, renames of empty files
cause it to explode (this is fairly common with __init__.py).

+       status = RunShell("hg st", args)

If you call this on a renamed (or possibly deleted) file, hg prints a message to
stderr if the file doesn't currently exist. This is arguably a bug in hg, but we
should probably at least file a bug with them.

It might be worth parsing out revision strings of the form -rN:M before you send
them to hg st. Not a big deal since you can just clone at an old revision,
though.

+    return content, status[0:4]

I might just be confused, but it seems like this is sending the first two
characters of the filename as part of the status. What's this trying to do?

Otherwise, looks quite good.

[Antoine Pitrou, 2008-09-06 12:46:24]

Perhaps this issue can be closed, there's a new patch at
http://codereview.appspot.com/4651 which doesn't seem to have the "bad content"
problem anymore (there was indeed a missing newline at the end of the uploaded
patch :-)).

[Antoine Pitrou, 2008-10-08 15:12:14]

Someone should close this issue as well.