Extension development / Am I doing this right?

[Andrew McClure]

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?

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 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.

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