{"thread":{"id":"13728","subject":"[PATCH] perl/Git.pm: add rev_parse method","startedAt":"2008-05-30T04:43:05Z","lastAt":"2008-06-02T13:51:18Z","messageCount":32,"participants":["Lea Wiemann","Petr Baudis","Junio C Hamano","Randal L. Schwartz","Johannes Schindelin","Miklos Vajna"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"78109","messageId":"1212122585-7350-1-git-send-email-LeWiemann@gmail.com","threadId":"13728","inReplyTo":null,"subject":"[PATCH] perl/Git.pm: add rev_parse method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-30T04:43:05Z","receivedAt":"2008-05-30T04:43:05Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"The rev_parse method translates a revision name to a SHA1 hash, like\nthe git-rev-parse command.\n\nSigned-off-by: Lea Wiemann <LeWiemann@gmail.com>\n---\n\nThis is part of my work to extend Git.pm to create a usable\nrepository access abstraction layer for gitweb.\n\nI've tested this method by calling it with a few parameters, but\nthere's no test suite yet.  I'll probably send a message to the\nmailing list about testing Git.pm soon.\n\nThis is my first patch to Git.pm, so please let me know if there is\nanything wrong with it!\n\n perl/Git.pm |   22 ++++++++++++++++++++++\n 1 files changed, 22 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex d05b633..9ef8cb0 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -716,6 +716,28 @@ sub ident_person {\n \treturn \"$ident[0] <$ident[1]>\";\n }\n \n+=item rev_parse ( REVISION_NAME )\n+\n+Look up the specified revision name and return the SHA1 hash, or\n+return undef if the lookup failed.  See the git-rev-parse command.\n+\n+=cut\n+\n+sub rev_parse {\n+    # We could allow for a list of revisions here.\n+    my ($self, $rev_name) = @_;\n+\n+    my $hash;\n+    try {\n+        # The --default option works around rev-parse's lack of\n+        # support for getopt style \"--\" separators (it would fail for\n+        # tags named \"--foo\" without it).\n+        $hash = $self->command_oneline(\"rev-parse\", \"--verify\", \"--default\",\n+                                       $rev_name);\n+    } catch Git::Error::Command with { };\n+    return undef unless defined $hash and $hash =~ /^([0-9a-fA-F]{40})$/;\n+    $hash;\n+}\n \n =item hash_object ( TYPE, FILENAME )\n \n-- \n1.5.5.GIT\n"},{"id":"78111","messageId":"483FA6B3.4070607@gmail.com","threadId":"13728","inReplyTo":"1212122585-7350-1-git-send-email-LeWiemann@gmail.com","subject":"Re: [PATCH] perl/Git.pm: add rev_parse method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-30T07:03:15Z","receivedAt":"2008-05-30T07:03:15Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"I wrote:\n> [Commit:] The rev_parse method translates a revision name to a SHA1 hash, like\n> the git-rev-parse command.\n\nOh, here's one problem: I'll probably do a lot of changes to Git.pm, and \nit might be handy for me to be able to change my own methods later.   I \ndefinitely wouldn't like to see Git.pm end up in some release while I'm \nin the middle of a major refactoring.\n\nShould I perhaps stay on my branch with these changes, and then merge \nwhen it has stabilized (in 1-3 months)?\n\nOne thing I'd be concerned about is that I might introduce fundamental \nissues in my API, since I'm neither a Git nor a Perl expert (yet ^^). \nWhat's the best way to avoid discovering such issues only at the Big \nMerge?  Is there anyone who'd be willing to monitor my commits and give \nme feedback on a semi-continuous basis?\n\n(I'm working on this more-or-less fulltime as part of a Google Summer of \nCode project.)\n\nBest,\n\n     Lea\n"},{"id":"78123","messageId":"20080530095047.GD18781@machine.or.cz","threadId":"13728","inReplyTo":"1212122585-7350-1-git-send-email-LeWiemann@gmail.com","subject":"Re: [PATCH] perl/Git.pm: add rev_parse method","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-05-30T09:50:47Z","receivedAt":"2008-05-30T09:50:47Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Hi,\n\nOn Fri, May 30, 2008 at 06:43:05AM +0200, Lea Wiemann wrote:\n> diff --git a/perl/Git.pm b/perl/Git.pm\n> index d05b633..9ef8cb0 100644\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -716,6 +716,28 @@ sub ident_person {\n>  \treturn \"$ident[0] <$ident[1]>\";\n>  }\n>  \n> +=item rev_parse ( REVISION_NAME )\n\nI believe it would be more consistent to call it parse_rev() and in fact\nalso less confusing since this is not full-fledged rev-parse frontend\n(and probably should not be; rev-parse is terribly overloaded with much\nmore stuff).\n\n> +\n> +Look up the specified revision name and return the SHA1 hash, or\n> +return undef if the lookup failed.  See the git-rev-parse command.\n> +\n> +=cut\n> +\n> +sub rev_parse {\n> +    # We could allow for a list of revisions here.\n> +    my ($self, $rev_name) = @_;\n> +\n> +    my $hash;\n> +    try {\n> +        # The --default option works around rev-parse's lack of\n> +        # support for getopt style \"--\" separators (it would fail for\n> +        # tags named \"--foo\" without it).\n> +        $hash = $self->command_oneline(\"rev-parse\", \"--verify\", \"--default\",\n> +                                       $rev_name);\n> +    } catch Git::Error::Command with { };\n\nI think it is better style to use the regular pattern of checking\n$E->value() and either return undef right away or pass the exception\n(e.g. in case rev-parse cannot be executed for some reason).\n\n> +    return undef unless defined $hash and $hash =~ /^([0-9a-fA-F]{40})$/;\n\nWhen can this trigger?\n\n> +    $hash;\n> +}\n>  \n>  =item hash_object ( TYPE, FILENAME )\n>  \n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nWhatever you can do, or dream you can, begin it.\nBoldness has genius, power, and magic in it.\t-- J. W. von Goethe\n"},{"id":"78125","messageId":"20080530095938.GE18781@machine.or.cz","threadId":"13728","inReplyTo":"483FA6B3.4070607@gmail.com","subject":"Re: [PATCH] perl/Git.pm: add rev_parse method","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-05-30T09:59:38Z","receivedAt":"2008-05-30T09:59:38Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Fri, May 30, 2008 at 09:03:15AM +0200, Lea Wiemann wrote:\n> I wrote:\n>> [Commit:] The rev_parse method translates a revision name to a SHA1 hash, \n>> like\n>> the git-rev-parse command.\n>\n> Oh, here's one problem: I'll probably do a lot of changes to Git.pm, and it \n> might be handy for me to be able to change my own methods later.   I \n> definitely wouldn't like to see Git.pm end up in some release while I'm in \n> the middle of a major refactoring.\n>\n> Should I perhaps stay on my branch with these changes, and then merge when \n> it has stabilized (in 1-3 months)?\n\nI have two proposals:\n\n(i) Tell Junio you would like the changes to stay in pu or next for now.\nBut he will probably do that by default anyway. :-) Thus, you do not\nneed to worry about them getting into a release any soon, but you will\nstill see some real-life testing (in theory).\n\n(ii) When introducing new interface, introduce a user to it right away.\nSo, if I were you, my roadmap would be something like:\n\n\t(a) Make Git.pm use gitweb's config parser\n\t(b) Add 'use Git' to gitweb and convert all Git calls possible\n\t(c) For the rest, introduce the necessary methods to Git.pm,\n\t    one patch per method (I would even bundle the Git.pm\n\t    addition with the gitweb changes)\n\nI'm not saying you _have_ to do it this way, but I believe it's one of\nthe smoothest paths. Me and I think many others of the Git project are\nhuge believers in \"small steps\" instead of \"grand designs\"; you will be\ngetting immediate feedback about your changes as you go and your work\nwill be useful at any point of time.\n\n> One thing I'd be concerned about is that I might introduce fundamental \n> issues in my API, since I'm neither a Git nor a Perl expert (yet ^^). \n> What's the best way to avoid discovering such issues only at the Big Merge? \n> Is there anyone who'd be willing to monitor my commits and give me \n> feedback on a semi-continuous basis?\n\n(I would love to provide feedback and review your patches, but please\nnote that at the same time I cannot promise I will be able to do it all\nthe time; I have failed these hopes in the past already, and again\nlikely will in the future.  But hopefully there's plenty of other Perl\nhackers on the list.)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nWhatever you can do, or dream you can, begin it.\nBoldness has genius, power, and magic in it.\t-- J. W. von Goethe\n"},{"id":"78148","messageId":"48401A09.6060301@gmail.com","threadId":"13728","inReplyTo":"20080530095938.GE18781@machine.or.cz","subject":"Merging strategy for extending Git.pm (was: [PATCH] perl/Git.pm: add rev_parse method)","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-30T15:15:21Z","receivedAt":"2008-05-30T15:15:21Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Petr Baudis wrote:\n> (i) Tell Junio you would like the changes to stay in pu or next for now.\n> [...]\n> you will be getting immediate feedback about your changes as you go and \n> your work will be useful at any point of time.\n\nJunio, any comments on this?  Doing doing small iterations is great IMO, \nand I'll be doing it in any case.  However, right now I see three \npossible problems with merging to pu continuously (rather than keeping \nit on my branch and merging in large chunks):\n\n1. I'm working full-time on this, so I might produce patches that \nloosely depend on one another at a peak rate of 2-3 per day.  (I can do \nother project-related stuff while my patches are waiting for review, but \nonly so much of course.)  Do you have any experience with working with \nfull-time developers on Git?  Do you see problems with my potentially \nhigh patch frequency?\n\n2. I'll be changing my own API.  In other words, the API is really \nunstable while I work on this (with the only user of the API being \nGitweb, which I'll update as I go).  Is that OK for the pu branch?\n\n3. I try to be careful with my commits, but it might still cause more \nwork for whoever reviews my patches, compared to reviewing larger \nchunks.  (That's because some of the stuff I write might end up being \ndeleted or rewritten later.)\n\nApart from that, I think merging (and getting feedback) continuously is \ngreat.\n\nSo, comments would be much appreciated!\n\n-- Lea\n"},{"id":"78167","messageId":"1212179270-26170-1-git-send-email-LeWiemann@gmail.com","threadId":"13728","inReplyTo":"20080530095047.GD18781@machine.or.cz","subject":"[PATCH] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-30T20:27:50Z","receivedAt":"2008-05-30T20:27:50Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"The parse_rev method takes a revision name and returns a SHA1 hash,\nlike the git-rev-parse command.\n\nSigned-off-by: Lea Wiemann <LeWiemann@gmail.com>\n---\nHi Petr,\n\nThis patch incorporates all your suggestions.  Thanks for your help on\nIRC!\n\n perl/Git.pm |   38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex d05b633..4bc3604 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -716,6 +716,44 @@ sub ident_person {\n \treturn \"$ident[0] <$ident[1]>\";\n }\n \n+=item parse_rev ( REVISION_NAME )\n+\n+Look up the specified revision name and return the SHA1 hash, or\n+return undef if the lookup failed.  See git rev-parse --help, section\n+\"Specifying Revisions\".\n+\n+=cut\n+\n+sub parse_rev {\n+    # We could allow for a list of revisions here.\n+    my ($self, $rev_name) = @_;\n+\n+    my $hash;\n+    try {\n+        # The --quiet --verify options cause git-rev-parse to fail\n+        # with exit status 1 (instead of 128) if the given revision\n+        # name is not found, which enables us to distinguish not-found\n+        # from serious errors.  The --default option works around\n+        # git-rev-parse's lack of support for getopt style \"--\"\n+        # separators (it would fail for tags named \"--foo\" without\n+        # it).\n+        $hash = $self->command_oneline(\"rev-parse\", \"--verify\", \"--quiet\",\n+                                       \"--default\", $rev_name);\n+    } catch Git::Error::Command with {\n+        my $E = shift;\n+        if ($E->value() == 1) {\n+            # Revision name not found.\n+            $hash = undef;\n+        } else {\n+            throw $E;\n+        }\n+    };\n+    # Guard against unexpected output.\n+    throw Error::Simple(\n+        \"parse_rev: unexpected output for \\\"$rev_name\\\": $hash\")\n+        if defined $hash and $hash !~ /^([0-9a-fA-F]{40})$/;\n+    return $hash;\n+}\n \n =item hash_object ( TYPE, FILENAME )\n \n-- \n1.5.5.GIT\n"},{"id":"78172","messageId":"7vve0vilj9.fsf@gitster.siamese.dyndns.org","threadId":"13728","inReplyTo":"483FA6B3.4070607@gmail.com","subject":"Re: [PATCH] perl/Git.pm: add rev_parse method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-30T20:28:42Z","receivedAt":"2008-05-30T20:28:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lea Wiemann <lewiemann@gmail.com> writes:\n\n> Should I perhaps stay on my branch with these changes, and then merge\n> when it has stabilized (in 1-3 months)?\n>\n> One thing I'd be concerned about is that I might introduce fundamental\n> issues in my API, since I'm neither a Git nor a Perl expert (yet\n> ^^). What's the best way to avoid discovering such issues only at the\n> Big Merge?\n\nFirst of all, we do not do \"Big Merge\".  We merge small and we merge\noften.  Nobody has perfect foresight, so you shouldn't be too afraid of\ncontaminating the public history with experiments that did not pan out\nwell.\n\n> Is there anyone who'd be willing to monitor my commits and\n> give me feedback on a semi-continuous basis?\n\nIsn't it what your GSoC mentor is for ;-)?\n\nYou can seek wider exposure in various different ways:\n\n * Send [RFC] patches to the list; that's how this community is supposed\n   to work, although I do not see as much reviews as I would personally\n   want to see from other people these days for some reason [*1*].\n\n   I may pick up \"next\" worthy ones to \"next\", and perhaps other ones to\n   \"pu\" as time permits.\n\n * Have your repository on repo.or.cz (I thought GSoC student project for\n   git were supposed to be hosted there?)  People interested in Perl\n   interface in general and Gitweb in particular can try your progress\n   out.\n\n\n[Footnote]\n\n*1* I suspect that maybe there is a misconception that patch submission\nand review on the list is a dialogue between the submitter and the\nmaintainer.  It is _NOT_ the case.  I'd rather stay back, sipping my\nCaipirinha, listening to _other_ people argue and improve the submitted\npatches, while occasionally giving some guidance to the course of the\ndiscussion.  And when the polished result emerges finally, apply it to my\ntree, taking all the credit.  _That_ is how the community is supposed to\nwork, isn't it? ;-)\n"},{"id":"78176","messageId":"20080530210531.GH18781@machine.or.cz","threadId":"13728","inReplyTo":"1212179270-26170-1-git-send-email-LeWiemann@gmail.com","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-05-30T21:05:31Z","receivedAt":"2008-05-30T21:05:31Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Fri, May 30, 2008 at 10:27:50PM +0200, Lea Wiemann wrote:\n> The parse_rev method takes a revision name and returns a SHA1 hash,\n> like the git-rev-parse command.\n> \n> Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>\n\nNice, this looks quite sane!\n\nAcked-by: Petr Baudis <pasky@suse.cz>\n\n\t\t\t\tPetr \"Pasky\" Baudis\n"},{"id":"78179","messageId":"7vd4n3iivt.fsf@gitster.siamese.dyndns.org","threadId":"13728","inReplyTo":"20080530210531.GH18781@machine.or.cz","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-30T21:25:58Z","receivedAt":"2008-05-30T21:25:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> On Fri, May 30, 2008 at 10:27:50PM +0200, Lea Wiemann wrote:\n>> The parse_rev method takes a revision name and returns a SHA1 hash,\n>> like the git-rev-parse command.\n>> \n>> Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>\n>\n> Nice, this looks quite sane!\n\nPerhaps, but except for the use of nonstandard try...catch.  I have been\nwondering if we can move away from it, with the goal of eventually getting\nrid of the construct altogether.\n\nDidn't we hear from Randal that the construct is known to be leaky?\n"},{"id":"78184","messageId":"86r6bjmpqc.fsf@blue.stonehenge.com","threadId":"13728","inReplyTo":"7vd4n3iivt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2008-05-30T21:44:27Z","receivedAt":"2008-05-30T21:44:27Z","isPatch":true,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Junio\" == Junio C Hamano <gitster@pobox.com> writes:\n\nJunio> Perhaps, but except for the use of nonstandard try...catch.  I have been\nJunio> wondering if we can move away from it, with the goal of eventually getting\nJunio> rid of the construct altogether.\n\nJunio> Didn't we hear from Randal that the construct is known to be leaky?\n\nIt'd be trivial to avoid this try/catch.\n\neval {\n  ... \n};\nif ($@) {\n  if (UNIVERSAL::isa($@, \"That::Class::Which::Should::Be::Ignored\")) {\n    # ignore it\n  } else {\n    die $@; # re-throw it\n  }\n}\n\nNo leak there.\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>\nSmalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.\nSee http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion\n"},{"id":"78186","messageId":"20080530214928.GK593@machine.or.cz","threadId":"13728","inReplyTo":"7vd4n3iivt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-05-30T21:49:28Z","receivedAt":"2008-05-30T21:49:28Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Fri, May 30, 2008 at 02:25:58PM -0700, Junio C Hamano wrote:\n> Perhaps, but except for the use of nonstandard try...catch.  I have been\n> wondering if we can move away from it, with the goal of eventually getting\n> rid of the construct altogether.\n\nIt's consistent with the rest of the code; we may want to remove it,\nbut we should either do it everywhere or not at all and I think it's\nan issue independent on this patch.\n\n> Didn't we hear from Randal that the construct is known to be leaky?\n\nI agree that in hindsight, using this construct was probably not the\nbest idea, and I'm in principle not against removing it as long as there\nis no functionality lost - the ability to let the program just fail\nif I don't care in my code, or catch specifically for nonzero exit codes\nand let the program fail in other cases, is very useful and I do use it\nin some of my code; I would hate to lose this flexibility.\n\nIf there is an alternative (perhaps even slightly more clumsy) way to\nsolve this, I have no problem with switching over; however, I don't have\nmuch time or interest to research this myself. If Lea wants to figure\nsomething out, she (*) is welcome as far as I'm concerned.\n\n\n(*) Lea, I hope I figured this out right - sorry if not. :-)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nWhatever you can do, or dream you can, begin it.\nBoldness has genius, power, and magic in it.\t-- J. W. von Goethe\n"},{"id":"78187","messageId":"484078C7.7020008@gmail.com","threadId":"13728","inReplyTo":"86r6bjmpqc.fsf@blue.stonehenge.com","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-30T21:59:35Z","receivedAt":"2008-05-30T21:59:35Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Randal L. Schwartz wrote:\n> [Move to eval/die.]  No leak there.\n\nI'm not an experienced Perl hacker, but I intuitively liked the \nthrow/catch method better than the eval/die method.\n\nIf I read \nhttp://www.nntp.perl.org/group/perl.perl5.porters/2006/03/msg110331.html \ncorrectly, this has been fixed in recent 5.8 versions, so it would \nreally only affect 5.6.  But perhaps we don't care about 5.6. ;-) \nPeople who run into serious memory issues with 5.6 will have to upgrade, \nand the existing functionality that programs like git-send-email or \ngit-svn rely on seems to work fine with the memory leaks, so using \nthrow/catch further probably won't cause any regressions for them.\n\nI'm honestly not too keen on sacrificing time (or code prettiness) on \n5.6 compatibility, so if there are no reasons besides the memory leak to \nmove away from throw/catch, perhaps we can just keep using it?\n\n-- Lea\n"},{"id":"78189","messageId":"86mym7mov6.fsf@blue.stonehenge.com","threadId":"13728","inReplyTo":"484078C7.7020008@gmail.com","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2008-05-30T22:03:09Z","receivedAt":"2008-05-30T22:03:09Z","isPatch":true,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Lea\" == Lea Wiemann <lewiemann@gmail.com> writes:\n\nLea> Randal L. Schwartz wrote:\n>> [Move to eval/die.]  No leak there.\n\nLea> I'm not an experienced Perl hacker, but I intuitively liked the throw/catch\nLea> method better than the eval/die method.\n\nI *am* an experienced hacker, and every Perl hacker worth their salt\nunderstands that \"eval/die\" *means* try/catch from other languages.\n\nThe extra layer of syntactic sugar is *not* worth it.  It merely obfuscates.\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>\nSmalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.\nSee http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion\n"},{"id":"78190","messageId":"48407A37.4080603@gmail.com","threadId":"13728","inReplyTo":"86mym7mov6.fsf@blue.stonehenge.com","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-30T22:05:43Z","receivedAt":"2008-05-30T22:05:43Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Randal L. Schwartz wrote:\n> \"eval/die\" *means* try/catch from other languages.\n> \n> The extra layer of syntactic sugar is *not* worth it.  It merely obfuscates.\n\nCool, thanks for clarifying this.  As Petr suggested, this should \nprobably be fixed in a separate patch.  (Perhaps after we have a test \nsuite, to avoid regressions.)\n\n-- Lea\n"},{"id":"78192","messageId":"7vtzgfh1um.fsf@gitster.siamese.dyndns.org","threadId":"13728","inReplyTo":"484078C7.7020008@gmail.com","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-30T22:19:13Z","receivedAt":"2008-05-30T22:19:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lea Wiemann <lewiemann@gmail.com> writes:\n\n> I'm honestly not too keen on sacrificing time (or code prettiness) on\n> 5.6 compatibility, so if there are no reasons besides the memory leak\n> to move away from throw/catch, perhaps we can just keep using it?\n\nIf we aim for code prettyness, I would say try/catch should definitely go.\nThey are spelled as \"eval\" in properly written Perl, and not doing so\nmakes your program look ugly.\n"},{"id":"78198","messageId":"7vabi7gz0g.fsf@gitster.siamese.dyndns.org","threadId":"13728","inReplyTo":"48401A09.6060301@gmail.com","subject":"Re: Merging strategy for extending Git.pm","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-30T23:20:31Z","receivedAt":"2008-05-30T23:20:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lea Wiemann <lewiemann@gmail.com> writes:\n\n> 1. I'm working full-time on this, so I might produce patches that\n> loosely depend on one another at a peak rate of 2-3 per day.  (I can\n> do other project-related stuff while my patches are waiting for\n> review, but only so much of course.)  Do you have any experience with\n> working with full-time developers on Git?  Do you see problems with my\n> potentially high patch frequency?\n\nAs long as experienced people on the list review patches, I do not see a\nproblem.\n\n    There seems to be a misconception (not you particularly but some parts\n    of the list audience in general) that a review cycle of a patch sent\n    to the list is a dialog between the submitter and me.  It is _NOT_.\n    If it has rooms for improvements, other people would help you polish\n    it, which happens often.  However, if it is very well done, I'd also\n    like other people to say so after reviewing more often.\n\n> 2. I'll be changing my own API.  In other words, the API is really\n> unstable while I work on this (with the only user of the API being\n> Gitweb, which I'll update as I go).  Is that OK for the pu branch?\n\nIt does not _have_ to even land on 'pu'.  Sending the patches to the list,\nasking interesting parties to look at them, _and_ having people actually\nreview them is more valuable part.\n\nIf your series will become big, I do not mind (re)merging it from time to\ntime in 'pu' to give it a wider exposure.  If you want to go this route,\nyou would have a publicly fetchable repository of your own to house your\nchanges (repo.or.cz?).\n\nI do not even mind merging the branch to 'next' if the series is 'next'\nworthy, but that places heavier responsibility on your part to keep the\nhistory of that branch clean.\n\n> 3. I try to be careful with my commits, but it might still cause more\n> work for whoever reviews my patches, compared to reviewing larger\n> chunks.  (That's because some of the stuff I write might end up being\n> deleted or rewritten later.)\n\nOne thing you could do, when sending out [PATCH v$n] for the value of $n\ngreater than 1, is to mention what the improvements are since the previous\nround in the message (typically after the three-dash separator).  This\nhelps reviewers who have already seen your previous iteration.  The full\nrationale of the change needs to be kept in the proposed commit log\nmessage.\n\nFor example, your first patch may look like this:\n\n\tSubject: [PATCH] Git.pm: Add rev_parse() sub\n\n\tThis adds a rev_parse() sub to return the 40-byte object name\n        from given \"extended SHA-1\" expression.\n\n\tSigned-off-by: A U Thor <au.thor@example.com>\n\t---\n         <<diffstat and patch>>\n\n\nThen after Pasky and others suggest improvements, your second message will\nappear on the list:\n\n\tSubject: [PATCH v2] Git.pm: Add parse_rev()\n\n\tThis adds 'parse_rev()' sub to return the 40-byte object name from\n        given \"extended SHA-1\" expression.  It returns undef if the given\n        string is malformed.\n\n\tSigned-off-by: A U Thor <au.thor@example.com>\n\t---\n\n         Changes relative to v1 are:\n         \n         * Fixed indentation;\n         * Use -q to squelch non-fatal errors so that they do not leak\n           to the STDERR;\n         * Improved in-code documentation.\n\n\t <<diffstat and patch>>\n\nSome people also sends an interdiff between v$n-1 and v$n as a separate\nmessage, and it helps reviewing when the change is big.\n"},{"id":"78217","messageId":"alpine.DEB.1.00.0805311237100.13507@racer.site.net","threadId":"13728","inReplyTo":"48401A09.6060301@gmail.com","subject":"Re: Merging strategy for extending Git.pm (was: [PATCH] perl/Git.pm: add rev_parse method)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-31T11:38:51Z","receivedAt":"2008-05-31T11:38:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 30 May 2008, Lea Wiemann wrote:\n\n> 2. I'll be changing my own API.  In other words, the API is really \n>    unstable while I work on this (with the only user of the API being \n>    Gitweb, which I'll update as I go).\n\nI think it would be best for you to have that in a personal fork of git on \nrepo.or.cz.  It is really easy to set up, and working with you will be \neasier, since one can always \"git fetch\" your newest stuff.\n\nThen, I would recommend making many many small patches.  You can always \nreorder/squash them with \"git rebase -i\" later, and you will probably need \nto rebase them to newer upstream revisions anyway.\n\nCiao,\nDscho\n"},{"id":"78218","messageId":"alpine.DEB.1.00.0805311248050.13507@racer.site.net","threadId":"13728","inReplyTo":"484078C7.7020008@gmail.com","subject":"Re: [PATCH] perl/Git.pm: add parse_rev method","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-31T11:50:14Z","receivedAt":"2008-05-31T11:50:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 30 May 2008, Lea Wiemann wrote:\n\n> I'm honestly not too keen on sacrificing time (or code prettiness) on \n> 5.6 compatibility, so if there are no reasons besides the memory leak to \n> move away from throw/catch, perhaps we can just keep using it?\n\nI think your opinion would change dramatically if you were stuck on a \nplatform with Perl 5.6.  In general, I deem it not nice to sacrifice \nbackwards compatibility just because _you_ do not need it.\n\nI mean, by that argument we could scrap the whole Git UI and rewrite it \nanew: a lot of compatibility warts would Just Go Away.\n\nCiao,\nDscho\n"},{"id":"78219","messageId":"20080531121755.GL593@machine.or.cz","threadId":"13728","inReplyTo":"alpine.DEB.1.00.0805311248050.13507@racer.site.net","subject":"Support for old Perl versions","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-05-31T12:17:55Z","receivedAt":"2008-05-31T12:17:55Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"  Hi,\n\nOn Sat, May 31, 2008 at 12:50:14PM +0100, Johannes Schindelin wrote:\n> On Fri, 30 May 2008, Lea Wiemann wrote:\n> \n> > I'm honestly not too keen on sacrificing time (or code prettiness) on \n> > 5.6 compatibility, so if there are no reasons besides the memory leak to \n> > move away from throw/catch, perhaps we can just keep using it?\n> \n> I think your opinion would change dramatically if you were stuck on a \n> platform with Perl 5.6.  In general, I deem it not nice to sacrifice \n> backwards compatibility just because _you_ do not need it.\n\n  let's get some perspective here: 5.6.1 was released on 2001-Apr-08.\n5.8.0 followed on 2002-Jul-18.  Is there anyone on the list who _is_\nstuck on a platform with Perl 5.6 _and_ uses Git on it? Heck, we\nare even approaching GNU Interactive Tools 4.3.20 release here,\nwalking that much back.\n\n  Of course, there's no sense in arbitrarily requiring newer Perl\nversions until we know we are not compatible anymore, but frankly,\nI don't think wasting time on being compatible with 5.6 is worth it\neither unless its actual users speak up.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nWhatever you can do, or dream you can, begin it.\nBoldness has genius, power, and magic in it.\t-- J. W. von Goethe\n"},{"id":"78220","messageId":"alpine.DEB.1.00.0805311328040.13507@racer.site.net","threadId":"13728","inReplyTo":"20080531121755.GL593@machine.or.cz","subject":"Re: Support for old Perl versions","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-31T12:32:16Z","receivedAt":"2008-05-31T12:32:16Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 31 May 2008, Petr Baudis wrote:\n\n> On Sat, May 31, 2008 at 12:50:14PM +0100, Johannes Schindelin wrote:\n> > On Fri, 30 May 2008, Lea Wiemann wrote:\n> > \n> > > I'm honestly not too keen on sacrificing time (or code prettiness) \n> > > on 5.6 compatibility, so if there are no reasons besides the memory \n> > > leak to move away from throw/catch, perhaps we can just keep using \n> > > it?\n> > \n> > I think your opinion would change dramatically if you were stuck on a \n> > platform with Perl 5.6.  In general, I deem it not nice to sacrifice \n> > backwards compatibility just because _you_ do not need it.\n> \n>   let's get some perspective here: 5.6.1 was released on 2001-Apr-08. \n> 5.8.0 followed on 2002-Jul-18.  Is there anyone on the list who _is_ \n> stuck on a platform with Perl 5.6 _and_ uses Git on it? Heck, we are \n> even approaching GNU Interactive Tools 4.3.20 release here, walking that \n> much back.\n\nI think this is not an interesting question.  Those stuck with Perl 5.6 \nare most likely not those who lurk on this list.\n\nSure, we could just require users to upgrade to Linux, newest glibc and \neverything and be done.  We could also require our users to stick their \nfingers where the sun don't shine.\n\nThe really interesting question is: is the time of a single developer (who \ngets all the upsides of requiring a certain setup) worth the hassle and \npain of possibly more than one person getting all the _downsides_?\n\nIn the case that started this thread, I would not hesitate a single \nmicrosecond to answer \"No, hell no\".\n\nHth,\nDscho\n"},{"id":"78222","messageId":"484147C7.1030001@gmail.com","threadId":"13728","inReplyTo":"alpine.DEB.1.00.0805311237100.13507@racer.site.net","subject":"Re: Merging strategy for extending Git.pm","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-31T12:42:47Z","receivedAt":"2008-05-31T12:42:47Z","isPatch":false,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Johannes Schindelin wrote:\n> I think it would be best for you to have that in a personal fork of git on \n> repo.or.cz.\n\nThanks Junio and Johannes for you comments!  My repository is here:\nhttp://repo.or.cz/w/git/gitweb-caching.git\n\n-- Lea\n"},{"id":"78223","messageId":"alpine.DEB.1.00.0805311351530.13507@racer.site.net","threadId":"13728","inReplyTo":"484147C7.1030001@gmail.com","subject":"Re: Merging strategy for extending Git.pm","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-31T12:52:00Z","receivedAt":"2008-05-31T12:52:00Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 31 May 2008, Lea Wiemann wrote:\n\n> Johannes Schindelin wrote:\n> > I think it would be best for you to have that in a personal fork of \n> > git on repo.or.cz.\n> \n> Thanks Junio and Johannes for you comments!  My repository is here: \n> http://repo.or.cz/w/git/gitweb-caching.git\n\nNice!\n\nCiao,\nDscho\n"},{"id":"78225","messageId":"1212241932-28605-1-git-send-email-LeWiemann@gmail.com","threadId":"13728","inReplyTo":"1212122585-7350-1-git-send-email-LeWiemann@gmail.com","subject":"[PATCH v2] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-31T13:52:12Z","receivedAt":"2008-05-31T13:52:12Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"The parse_rev method takes a revision name and returns a SHA1 hash,\nlike the git-rev-parse command.\n\nSigned-off-by: Lea Wiemann <LeWiemann@gmail.com>\n---\nUse tabs instead of blanks to match the rest of the file.  No changes\napart from that.\n\n perl/Git.pm |   38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex d05b633..249daad 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -716,6 +716,44 @@ sub ident_person {\n \treturn \"$ident[0] <$ident[1]>\";\n }\n \n+=item parse_rev ( REVISION_NAME )\n+\n+Look up the specified revision name and return the SHA1 hash, or\n+return undef if the lookup failed.  See git rev-parse --help, section\n+\"Specifying Revisions\".\n+\n+=cut\n+\n+sub parse_rev {\n+\t# We could allow for a list of revisions here.\n+\tmy ($self, $rev_name) = @_;\n+\n+\tmy $hash;\n+\ttry {\n+\t\t# The --quiet --verify options cause git-rev-parse to fail\n+\t\t# with exit status 1 (instead of 128) if the given revision\n+\t\t# name is not found, which enables us to distinguish not-found\n+\t\t# from serious errors.  The --default option works around\n+\t\t# git-rev-parse's lack of support for getopt style \"--\"\n+\t\t# separators (it would fail for tags named \"--foo\" without\n+\t\t# it).\n+\t\t$hash = $self->command_oneline(\"rev-parse\", \"--verify\", \"--quiet\",\n+\t\t\t\t\t       \"--default\", $rev_name);\n+\t} catch Git::Error::Command with {\n+\t\tmy $E = shift;\n+\t\tif ($E->value() == 1) {\n+\t\t\t# Revision name not found.\n+\t\t\t$hash = undef;\n+\t\t} else {\n+\t\t\tthrow $E;\n+\t\t}\n+\t};\n+\t# Guard against unexpected output.\n+\tthrow Error::Simple(\n+\t\t\"parse_rev: unexpected output for \\\"$rev_name\\\": $hash\")\n+\t    if defined $hash and $hash !~ /^([0-9a-fA-F]{40})$/;\n+\treturn $hash;\n+}\n \n =item hash_object ( TYPE, FILENAME )\n \n-- \n1.5.5.GIT\n"},{"id":"78264","messageId":"1212290243-19393-1-git-send-email-LeWiemann@gmail.com","threadId":"13728","inReplyTo":"1212241932-28605-1-git-send-email-LeWiemann@gmail.com","subject":"[PATCH v3] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-01T03:17:22Z","receivedAt":"2008-06-01T03:17:22Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"The parse_rev method takes a revision name and returns a SHA1 hash,\nlike the git-rev-parse command.\n\nSigned-off-by: Lea Wiemann <LeWiemann@gmail.com>\n---\nChanges since PATCH v2: Updated the documentation (found this while\ntesting).  Here's the diff excerpt from v2 to v3:\n\n =item parse_rev ( REVISION_NAME )\n\n Look up the specified revision name and return the SHA1 hash, or\n-return undef if the lookup failed.  See git rev-parse --help, section\n-\"Specifying Revisions\".\n+return undef if the lookup failed.  When passed a SHA1 hash, always\n+return it even if it doesn't exist in the repository.\n+\n+See git rev-parse --help, section \"Specifying Revisions\", for valid\n+revision name formats.\n\n perl/Git.pm |   41 +++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 41 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex d05b633..80f7669 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -716,6 +716,47 @@ sub ident_person {\n \treturn \"$ident[0] <$ident[1]>\";\n }\n \n+=item parse_rev ( REVISION_NAME )\n+\n+Look up the specified revision name and return the SHA1 hash, or\n+return undef if the lookup failed.  When passed a SHA1 hash, always\n+return it even if it doesn't exist in the repository.\n+\n+See git rev-parse --help, section \"Specifying Revisions\", for valid\n+revision name formats.\n+\n+=cut\n+\n+sub parse_rev {\n+\t# We could allow for a list of revisions here.\n+\tmy ($self, $rev_name) = @_;\n+\n+\tmy $hash;\n+\ttry {\n+\t\t# The --quiet --verify options cause git-rev-parse to fail\n+\t\t# with exit status 1 (instead of 128) if the given revision\n+\t\t# name is not found, which enables us to distinguish not-found\n+\t\t# from serious errors.  The --default option works around\n+\t\t# git-rev-parse's lack of support for getopt style \"--\"\n+\t\t# separators (it would fail for tags named \"--foo\" without\n+\t\t# it).\n+\t\t$hash = $self->command_oneline(\"rev-parse\", \"--verify\", \"--quiet\",\n+\t\t\t\t\t       \"--default\", $rev_name);\n+\t} catch Git::Error::Command with {\n+\t\tmy $E = shift;\n+\t\tif ($E->value() == 1) {\n+\t\t\t# Revision name not found.\n+\t\t\t$hash = undef;\n+\t\t} else {\n+\t\t\tthrow $E;\n+\t\t}\n+\t};\n+\t# Guard against unexpected output.\n+\tthrow Error::Simple(\n+\t\t\"parse_rev: unexpected output for \\\"$rev_name\\\": $hash\")\n+\t    if defined $hash and $hash !~ /^([0-9a-fA-F]{40})$/;\n+\treturn $hash;\n+}\n \n =item hash_object ( TYPE, FILENAME )\n \n-- \n1.5.5.GIT\n"},{"id":"78265","messageId":"1212290243-19393-2-git-send-email-LeWiemann@gmail.com","threadId":"13728","inReplyTo":"1212290243-19393-1-git-send-email-LeWiemann@gmail.com","subject":"[PATCH v3] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-01T03:17:23Z","receivedAt":"2008-06-01T03:17:23Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"The parse_rev method takes a revision name and returns a SHA1 hash,\nlike the git-rev-parse command.\n\nSigned-off-by: Lea Wiemann <LeWiemann@gmail.com>\n---\nChanges since PATCH v2: Updated the documentation (found this while\ntesting).  Here's the diff excerpt from v2 to v3:\n\n =item parse_rev ( REVISION_NAME )\n\n Look up the specified revision name and return the SHA1 hash, or\n-return undef if the lookup failed.  See git rev-parse --help, section\n-\"Specifying Revisions\".\n+return undef if the lookup failed.  When passed a SHA1 hash, always\n+return it even if it doesn't exist in the repository.\n+\n+See git rev-parse --help, section \"Specifying Revisions\", for valid\n+revision name formats.\n\n perl/Git.pm |   41 +++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 41 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex d05b633..80f7669 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -716,6 +716,47 @@ sub ident_person {\n \treturn \"$ident[0] <$ident[1]>\";\n }\n \n+=item parse_rev ( REVISION_NAME )\n+\n+Look up the specified revision name and return the SHA1 hash, or\n+return undef if the lookup failed.  When passed a SHA1 hash, always\n+return it even if it doesn't exist in the repository.\n+\n+See git rev-parse --help, section \"Specifying Revisions\", for valid\n+revision name formats.\n+\n+=cut\n+\n+sub parse_rev {\n+\t# We could allow for a list of revisions here.\n+\tmy ($self, $rev_name) = @_;\n+\n+\tmy $hash;\n+\ttry {\n+\t\t# The --quiet --verify options cause git-rev-parse to fail\n+\t\t# with exit status 1 (instead of 128) if the given revision\n+\t\t# name is not found, which enables us to distinguish not-found\n+\t\t# from serious errors.  The --default option works around\n+\t\t# git-rev-parse's lack of support for getopt style \"--\"\n+\t\t# separators (it would fail for tags named \"--foo\" without\n+\t\t# it).\n+\t\t$hash = $self->command_oneline(\"rev-parse\", \"--verify\", \"--quiet\",\n+\t\t\t\t\t       \"--default\", $rev_name);\n+\t} catch Git::Error::Command with {\n+\t\tmy $E = shift;\n+\t\tif ($E->value() == 1) {\n+\t\t\t# Revision name not found.\n+\t\t\t$hash = undef;\n+\t\t} else {\n+\t\t\tthrow $E;\n+\t\t}\n+\t};\n+\t# Guard against unexpected output.\n+\tthrow Error::Simple(\n+\t\t\"parse_rev: unexpected output for \\\"$rev_name\\\": $hash\")\n+\t    if defined $hash and $hash !~ /^([0-9a-fA-F]{40})$/;\n+\treturn $hash;\n+}\n \n =item hash_object ( TYPE, FILENAME )\n \n-- \n1.5.5.GIT\n"},{"id":"78294","messageId":"4842DE78.7000006@gmail.com","threadId":"13728","inReplyTo":"1212290243-19393-2-git-send-email-LeWiemann@gmail.com","subject":"Re: [PATCH v3] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-01T17:38:00Z","receivedAt":"2008-06-01T17:38:00Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Lea Wiemann wrote:\n> Re: [PATCH v3] perl/Git.pm: add parse_rev method\n>\n> The parse_rev method takes a revision name and returns a SHA1 hash,\n> like the git-rev-parse command.\n\nI just discovered that you can also pass tree and blob identifiers to \ngit-rev-parse, like <commit>:<path>.\n\nHence, perhaps this method would be more appropriately named get_hash \n(or get_sha1), given that you can pass in things other than revisions. \nI'll probably post new patch versions soon.\n\nTerminology question: Is there *any* kind of agreed-on name for the \nidentifiers you pass into git-rev-parse (like HEAD^2 or \nmaster:test/foo.txt)?  I called it \"revision name\" before, but that's \nwrong for the \"...:<path>\" syntaxes, and \"object identifier\" is reserved \nfor hashes only (according to the glossary).  If there no better \nsuggestions, I'll probably go for \"extended identifiers\", since \nrev-parse --help calls this the \"extended SHA1 syntax\", and it also \nseems to be an unused term.\n\n-- Lea\n"},{"id":"78312","messageId":"20080601215449.GC29404@genesis.frugalware.org","threadId":"13728","inReplyTo":"4842DE78.7000006@gmail.com","subject":"Re: [PATCH v3] perl/Git.pm: add parse_rev method","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-06-01T21:54:49Z","receivedAt":"2008-06-01T21:54:49Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sun, Jun 01, 2008 at 07:38:00PM +0200, Lea Wiemann <lewiemann@gmail.com> wrote:\n> Terminology question: Is there *any* kind of agreed-on name for the \n> identifiers you pass into git-rev-parse (like HEAD^2 or \n> master:test/foo.txt)?  I called it \"revision name\" before, but that's wrong \n> for the \"...:<path>\" syntaxes, and \"object identifier\" is reserved for \n> hashes only (according to the glossary).  If there no better suggestions, \n> I'll probably go for \"extended identifiers\", since rev-parse --help calls \n> this the \"extended SHA1 syntax\", and it also seems to be an unused term.\n\n`man git-rev-parse` calls them \"revisions\". Yes, even the commit:path\nones. So I would use \"revision\", or if you introduce a new term, I would\nstrongly suggest updating not just the glossary but git-rev-parse.txt as\nwell.\n\nThanks.\n"},{"id":"78317","messageId":"484327FB.8080105@gmail.com","threadId":"13728","inReplyTo":"20080601215449.GC29404@genesis.frugalware.org","subject":"Re: [PATCH v3] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-01T22:51:39Z","receivedAt":"2008-06-01T22:51:39Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Miklos Vajna wrote:\n> Lea Wiemann <lewiemann@gmail.com> wrote:\n>> Is there *any* name for the identifiers you pass into git-rev-parse\n>> (like HEAD^2 or master:test/foo.txt)?\n> \n> `man git-rev-parse` calls them \"revisions\". Yes, even the commit:path\n> ones.\n\nTrue -- it's quite cringeworthy indeed. ;)  As long as it only affects \nthe documentation for that particular method, I'll go with \"revision\".\n\n-- Lea\n"},{"id":"78318","messageId":"1212361784-20409-1-git-send-email-LeWiemann@gmail.com","threadId":"13728","inReplyTo":"1212290243-19393-2-git-send-email-LeWiemann@gmail.com","subject":"[PATCH v4] perl/Git.pm: add get_hash method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-01T23:09:44Z","receivedAt":"2008-06-01T23:09:44Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"The get_hash method takes a \"revision\" (as described in git-rev-parse\n--help) and returns the SHA1 hash of the commit, tree, file, or tag\nobject it refers to.\n\nSigned-off-by: Lea Wiemann <LeWiemann@gmail.com>\n---\nChanges since v3: Renamed parse_rev to get_hash, improved\ndocumentation to point out that the parameter can refer to file, tree\nand tag object as well, and added example in \"Synopsis\" section.\n\n perl/Git.pm |   45 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 45 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 97e61ef..f2e9e29 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -43,6 +43,9 @@ $VERSION = '0.01';\n   my $tempfile = tempfile();\n   my $size = $repo->cat_blob($sha1, $tempfile);\n \n+  my $head_commit_sha1 = $repo->get_hash('HEAD');  # see git-rev-parse --help\n+  my $file_blob_sha1   = $repo->get_hash('HEAD:path/file.txt');\n+\n =cut\n \n \n@@ -716,6 +719,48 @@ sub ident_person {\n \treturn \"$ident[0] <$ident[1]>\";\n }\n \n+=item get_hash ( REVISION )\n+\n+Look up the object referred to by C<REVISION> and return its SHA1\n+hash, or return undef if the lookup failed.  When passed a SHA1 hash,\n+always return it even if it doesn't exist in the repository.\n+\n+Note that C<REVISION> can refer to a commit, file, tree, or tag\n+object!  See git rev-parse --help, section \"Specifying Revisions\", for\n+valid formats of the C<REVISION> parameter.\n+\n+=cut\n+\n+sub get_hash {\n+\t# We could allow for a list of revisions here.\n+\tmy ($self, $rev_name) = @_;\n+\n+\tmy $hash;\n+\ttry {\n+\t\t# The --quiet --verify options cause git-rev-parse to fail\n+\t\t# with exit status 1 (instead of 128) if the given revision\n+\t\t# name is not found, which enables us to distinguish not-found\n+\t\t# from serious errors.  The --default option works around\n+\t\t# git-rev-parse's lack of support for getopt style \"--\"\n+\t\t# separators (it would fail for tags named \"--foo\" without\n+\t\t# it).\n+\t\t$hash = $self->command_oneline(\"rev-parse\", \"--verify\", \"--quiet\",\n+\t\t\t\t\t       \"--default\", $rev_name);\n+\t} catch Git::Error::Command with {\n+\t\tmy $E = shift;\n+\t\tif ($E->value() == 1) {\n+\t\t\t# Revision name not found.\n+\t\t\t$hash = undef;\n+\t\t} else {\n+\t\t\tthrow $E;\n+\t\t}\n+\t};\n+\t# Guard against unexpected output.\n+\tthrow Error::Simple(\n+\t\t\"get_hash: unexpected output for \\\"$rev_name\\\": $hash\")\n+\t    if defined $hash and $hash !~ /^([0-9a-fA-F]{40})$/;\n+\treturn $hash;\n+}\n \n =item hash_object ( TYPE, FILENAME )\n \n-- \n1.5.5.GIT\n"},{"id":"78320","messageId":"1212362693-22084-1-git-send-email-LeWiemann@gmail.com","threadId":"13728","inReplyTo":"1212361784-20409-1-git-send-email-LeWiemann@gmail.com","subject":"[PATCH v4] perl/Git.pm: add get_hash method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-01T23:24:52Z","receivedAt":"2008-06-01T23:24:52Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"The get_hash method takes a \"revision\" (as described in git-rev-parse\n--help) and returns the SHA1 hash of the commit, tree, file, or tag\nobject it refers to.\n\nSigned-off-by: Lea Wiemann <LeWiemann@gmail.com>\n---\nChanges since v3: Renamed parse_rev to get_hash, improved\ndocumentation to point out that the parameter can refer to file, tree\nand tag object as well, and added example in \"Synopsis\" section.\n\n perl/Git.pm |   45 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 45 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 97e61ef..f2e9e29 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -43,6 +43,9 @@ $VERSION = '0.01';\n   my $tempfile = tempfile();\n   my $size = $repo->cat_blob($sha1, $tempfile);\n \n+  my $head_commit_sha1 = $repo->get_hash('HEAD');  # see git-rev-parse --help\n+  my $file_blob_sha1   = $repo->get_hash('HEAD:path/file.txt');\n+\n =cut\n \n \n@@ -716,6 +719,48 @@ sub ident_person {\n \treturn \"$ident[0] <$ident[1]>\";\n }\n \n+=item get_hash ( REVISION )\n+\n+Look up the object referred to by C<REVISION> and return its SHA1\n+hash, or return undef if the lookup failed.  When passed a SHA1 hash,\n+always return it even if it doesn't exist in the repository.\n+\n+Note that C<REVISION> can refer to a commit, file, tree, or tag\n+object!  See git rev-parse --help, section \"Specifying Revisions\", for\n+valid formats of the C<REVISION> parameter.\n+\n+=cut\n+\n+sub get_hash {\n+\t# We could allow for a list of revisions here.\n+\tmy ($self, $rev_name) = @_;\n+\n+\tmy $hash;\n+\ttry {\n+\t\t# The --quiet --verify options cause git-rev-parse to fail\n+\t\t# with exit status 1 (instead of 128) if the given revision\n+\t\t# name is not found, which enables us to distinguish not-found\n+\t\t# from serious errors.  The --default option works around\n+\t\t# git-rev-parse's lack of support for getopt style \"--\"\n+\t\t# separators (it would fail for tags named \"--foo\" without\n+\t\t# it).\n+\t\t$hash = $self->command_oneline(\"rev-parse\", \"--verify\", \"--quiet\",\n+\t\t\t\t\t       \"--default\", $rev_name);\n+\t} catch Git::Error::Command with {\n+\t\tmy $E = shift;\n+\t\tif ($E->value() == 1) {\n+\t\t\t# Revision name not found.\n+\t\t\t$hash = undef;\n+\t\t} else {\n+\t\t\tthrow $E;\n+\t\t}\n+\t};\n+\t# Guard against unexpected output.\n+\tthrow Error::Simple(\n+\t\t\"get_hash: unexpected output for \\\"$rev_name\\\": $hash\")\n+\t    if defined $hash and $hash !~ /^([0-9a-fA-F]{40})$/;\n+\treturn $hash;\n+}\n \n =item hash_object ( TYPE, FILENAME )\n \n-- \n1.5.5.GIT\n"},{"id":"78328","messageId":"7vk5h8a0up.fsf@gitster.siamese.dyndns.org","threadId":"13728","inReplyTo":"484327FB.8080105@gmail.com","subject":"Re: [PATCH v3] perl/Git.pm: add parse_rev method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-02T04:59:26Z","receivedAt":"2008-06-02T04:59:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lea Wiemann <lewiemann@gmail.com> writes:\n\n> Miklos Vajna wrote:\n>> Lea Wiemann <lewiemann@gmail.com> wrote:\n>>> Is there *any* name for the identifiers you pass into git-rev-parse\n>>> (like HEAD^2 or master:test/foo.txt)?\n>>\n>> `man git-rev-parse` calls them \"revisions\". Yes, even the commit:path\n>> ones.\n>\n> True -- it's quite cringeworthy indeed. ;)  As long as it only affects\n> the documentation for that particular method, I'll go with \"revision\".\n\nI think that is sensible, and the method can stay parse_rev, not get_hash,\ndon't you think?\n"},{"id":"78361","messageId":"4843FAD6.6020705@gmail.com","threadId":"13728","inReplyTo":"7vk5h8a0up.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] perl/Git.pm: add parse_rev method","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-02T13:51:18Z","receivedAt":"2008-06-02T13:51:18Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n> I think that is sensible, and the method can stay parse_rev, not get_hash,\n> don't you think?\n\nI'm not sure -- on the hand hand, parse_rev resembles \"rev-parse\" and \nmight be more intuitive for that reason.\n\nOn the other hand though, the mis-named \"revision\" arguments get passed \ninto many methods (right now get_hash, get_type, and cat_blob, but many \nmore to come).  Calling them \"revision\" everywhere is bound to cause \nmajor confusion, so I'll probably have to rename the arguments anyway. \nBut then, get_hash will be much more intuitive to understand than \nparse_rev since the API won't talk about \"revisions\" anywhere.\n\nAlso, get_hash indicates that it returns a hash and implies that it \ntakes an object name, but parse_rev indicates neither of those.  Since \nGit.pm doesn't mimic Git's command-line API anyway, I'd rather have \nclear method names.\n\n-- Lea\n"}]}