Extension development / Am I doing this right?

[Augie Fackler]

On Sep 3, 2011, at 12:49 PM, Andrew McClure wrote:
> 
> Hi! Comments inline:
> 
> On Sep 2, 2011, at 7:26 AM, Augie Fackler <lists at durin42.com> wrote:
> 
>> On Thu, Sep 1, 2011 at 2:45 AM, Andrew McClure <andrew.mcclure at gmail.com> wrote:
>>> Hello, I am doing some development on a mercurial extension. Because I have
>>> never done this before, I wanted to walk through what I'm doing and see if
>>> anyone had any advice about the "correct" way to do things. If this is the
>>> wrong place to ask questions about extension development please let me know.
>>> What I am doing is adding features to the existing hg-git plugin (my fork is
>>> at https://github.com/mcclure/hg-git). hg-git allows you to work on a
>>> translated "copy" of a remote git repository, and silently translates
>>> to/from git when you push and pull. My current patch attempts to address an
>>> inconvenience I've noticed with hg-git, where once the repository has been
>>> translated from git-ese to hg-ese all the revision hashes are different, and
>>> therefore it becomes difficult to collaborate with someone who is using git.
>>> (The other person says something about revision 7406c, you have no idea what
>>> that means, etc.) I made two specific changes to deal with this: First, if
>>> you say something like "hg up 7406c", and 7406c is a valid revision in the
>>> git version of your repository, it now treats "7406c" as an alias to the
>>> appropriate hg revision. Second, when you use a command which shows
>>> long-form information about a revision (like hg sum, hg log or hg outgoing)
>>> on an hg-git repository, it will say something like "git-rev: 7406c..."
>>> along with the list of bookmarks, tags etc corresponding to that revision.
>>> (My overall goal was to make addressing revisions by git-equivalent as
>>> seamless as addressing them by bookmark.) The patch works (in 1.9, I need to
>>> test it on earlier versions next), I'm just not sure if my implementation
>>> went about it right.
>>> The first change I'm pretty sure I did right. The extension does the trick
>>> of replacing the repository class with a subclass, and I just
>>> overloaded lookup(self, key). My version of lookup() calls the super
>>> lookup(); if error.RepoLookupError is thrown, it catches it, checks to see
>>> if it recognizes the unrecognized key, returns the appropriate revision if
>>> so, and re-raises the RepoLookupError if not. Does this sound right? Are
>>> there any potential pitfalls to expanding the list of revisions lookup()
>>> recognizes?
>> 
>> This sounds like a reasonable approach.
>> 
>>> The second change I'm more worried about. hg log and such, in the default
>>> implementation when no styles or templates are being used, just print out
>>> all the information line by line in a long
>>> function changeset_printer._show(). Since there was nowhere obvious to hook
>>> in in the middle of this function, I actually wound up subclassing ui,
>>> overloading write(), and watching for writes with the tag 'log.changeset'.
>>> When the changeset line is written, I parse its string to figure out the hg
>>> changeset being talked about at that moment and determine whether the line
>>> is formatted like an `hg log` or an `hg sum`, and then write() a second line
>>> containing the git-tag equivalent. This works, but seems kind of crazy!
>> 
>> This sounds somewhat gross. Perhaps we can wait on it for now?
> 
> Wait on the patch or wait on this particular detail?

This particular feature. In general I like one patch per feature anyway (note: I haven't looked at your patches at all, when you're ready for me to take a look please patchbomb them to the hg-git list).

> So: I do think this one feature is important. My case I was originally
> trying to solve was staring at the hg log in one window and staring at
> github.com in the other and not knowing how to match up A to B. If
> there aren't "incidental" git revisions in the interface like this
> then that case isn't served.

I wasn't disagreeing with you, but the particular approach is pretty, uh, scary.

> I will go ahead and look into whether there were better ways to
> implement the feature, however. One thing I'd wonder about is the
> template functionality I mentioned-- maybe it would make more sense to
> make the git-tag data available to templates, then implement a "style"
> or "template" that looks like the standard one and incidentally
> includes git-tags in sum and log. This would be less gross and also
> mean there would be a natural way for the user to turn the git-tag
> display off.
> 
> (One thing I'd say in favor of the patch's current approach-- although
> yeah, I did just talk it down-- is that it is conservative. If
> something unexpected happens, like the tag not being written or not
> being written with the exact format it expects, it does nothing. So I
> think the risk of problems should be in practice fairly small unless
> of course mercurial changes things after 1.9.)
> 
>> 
>>> Could this come to bite me in ways I didn't foresee, for example could it
>>> cause problems for someone not using the command line tool?
>>> Meanwhile, I'm a little confused about this "template" system. Since I
>>> overload the default (no style, no template) my changes to sum/log/etc
>>> simply quietly do nothing if you're using styles or custom templates. This
>>> is, I think, good, I shouldn't be messing with people's templates. However
>>> it seems like *if* someone *wanted* to write a custom template which was
>>> aware of hg-git, I should set up my hg-git additions so that they make the
>>> git-rev values visible to those templates. Here I'm totally lost. I have
>>> this vague sense that at some point some hashes get passed to the template
>>> and then processed with lines like {parent%changesetparent}, and I need to
>>> be either adding the git-rev strings to one of these hashes or to some kind
>>> of new hash, but I don't know where to do this or how to format it. What
>>> would be the sensible way of tacking one piece of template-visible
>>> information on to each changeset displayed inside a template?
>>> Thanks, any advice would be appreciated.
>> 
>> Not sure, I think Dan might have done something like this once for
>> hgsubversion, or at least looked into it.
>> 
> 
> Okay, I will try to look at the code for that.
> 
> Thanks!, and I will reply to your comments on the Google Group also.

You might be interested in my partially-baked hgit extension [0], which aims to work around most of these problems.

[0] http://bitbucket.org/durin42/hgit/

> 
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel at selenic.com
>>> http://selenic.com/mailman/listinfo/mercurial-devel
>>> 
>>>