{"thread":{"id":"30337","subject":"Git.pm","startedAt":"2012-04-26T04:15:09Z","lastAt":"2012-05-23T19:36:13Z","messageCount":24,"participants":["Subho Banerjee","Randal L. Schwartz","Tim Henigan","Junio C Hamano","Sam Vilain","Jonathan Nieder","demerphq","Andrew Sayers","Subho Sankar Banerjee"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"190092","messageId":"CAB3zAY3-Bn86bCr7Rxqi4vxbYFxUesLwm8gddxyMSexov2tOhw@mail.gmail.com","threadId":"30337","inReplyTo":null,"subject":"Git.pm","fromName":"Subho Banerjee","fromEmail":"subs.zero@gmail.com","sentAt":"2012-04-26T04:15:09Z","receivedAt":"2012-04-26T04:15:09Z","isPatch":false,"sender":{"key":"subs.zero@gmail.com","avatar":"https://gravatar.com/avatar/6159910e05d7650fc7da0a77c233711a014ea449cc5b30f0f065d0017b3357dd?d=mp&s=160"},"body":"Hello,\nI had made a proposal for the Git.pm project in GSoC. The proposal did\nnot get accepted, however, I see that no one in the GSoC accepted list\nis actually working on the Git.pm project. If some of you could give\nme some of your time in terms of advice on what exactly is needed for\nthe module, I am willing to work over the summer to get this module\nproduction-ready. I can probably put in 15-20 hours a week on this\nproject from May to August. I believe that will be enough time to\nroughly complete all that I had enumerated in my GSoC proposal. This\nof course will be strictly outside the GSoC framework.\n\nI plan to start coding on the project by 7th May and use the time from\nthen to now to investigate what code is there/ what is to be done etc.\nI had made an approximate timeline for the GSoC proposal and I would\nlike to follow it -\n---> [By 15th May] Get the current perl code, to use another mechanism\nof throwing errors(Try:Tiny)\n---> [By August] Get in place a more robust perl wrapper ie. expand\nthe code have a couple of more objects, Git::Repo, Git::Config etc.\n---> If all goes well, then by the beginning of August, get the perl\nmodule ready for CPANfication\n\nI also had a couple of questions -\n---> Do I base my code revisions on the master branch of the Git\ncodebase[https://github.com/git/git]? Or is there some other\nrepository which might be more recent.\n---> I saw gitweb-caching code from a previous GSoC project, the perl\nmodule there seems to have been developed beyond what is there in the\nmaster brach? However, these changes are atleast a couple of years old\nand havent been incorporated in the main codebase... Is there any\nparticular reason for that?\n---> I see in the code that it says that the API is experimental. Is\nthere any absolute need for backward compatibility, or can I try to\nredesign the API somewhat extensively?\n\nAlso, any suggestions and tips you can give me about the project will\nbe very helpful.\n\nCheers,\nSubho.\n"},{"id":"190133","messageId":"867gx2uyhf.fsf@red.stonehenge.com","threadId":"30337","inReplyTo":"CAB3zAY3-Bn86bCr7Rxqi4vxbYFxUesLwm8gddxyMSexov2tOhw@mail.gmail.com","subject":"Re: Git.pm","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2012-04-26T18:41:48Z","receivedAt":"2012-04-26T18:41:48Z","isPatch":false,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Subho\" == Subho Banerjee writes:\n\nSubho> Also, any suggestions and tips you can give me about the project will\nSubho> be very helpful.\n\nI offer my services for code-review and consultation.\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n \nSmalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.\nSee http://methodsandmessages.posterous.com/ for Smalltalk discussion\n"},{"id":"190134","messageId":"CAFouetgwRpB1GFJOC8PTVryVY-94S3xa5ZiSaWQWoz070qQ-6g@mail.gmail.com","threadId":"30337","inReplyTo":"CAB3zAY3-Bn86bCr7Rxqi4vxbYFxUesLwm8gddxyMSexov2tOhw@mail.gmail.com","subject":"Re: Git.pm","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-04-26T18:58:37Z","receivedAt":"2012-04-26T18:58:37Z","isPatch":false,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"On Thu, Apr 26, 2012 at 12:15 AM, Subho Banerjee wrote:\n>\n> ---> I see in the code that it says that the API is experimental. Is\n> there any absolute need for backward compatibility, or can I try to\n> redesign the API somewhat extensively?\n\nA quick grep of the code in 'master' shows Git.pm used in the following:\n\n - contrib/examples/git-remote.perl\n - git-add--interactive.perl\n - git-cvsexportcommit.perl\n - git-send-email.perl\n - git-svn.perl\n - t/perf/aggregate.perl\n\nThere is also work in progress on 'pu' that relies on Git.pm.\n\nBreaking any of these scripts would be bad. You may be able to\nrefactor them at the same time Git.pm is modified, but it would be\nwise to contact the authors before making any major changes.\n"},{"id":"190136","messageId":"xmqqy5pi8fq9.fsf@junio.mtv.corp.google.com","threadId":"30337","inReplyTo":"CAB3zAY3-Bn86bCr7Rxqi4vxbYFxUesLwm8gddxyMSexov2tOhw@mail.gmail.com","subject":"Re: Git.pm","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-26T19:17:50Z","receivedAt":"2012-04-26T19:17:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Subho Banerjee writes:\n\n> ... If some of you could give\n> me some of your time in terms of advice on what exactly is needed for\n> the module, I am willing to work over the summer to get this module\n> production-ready.\n\nWell, that sounds as if the module is currently not production ready,\nbut it has been used in the wild for quite a long time.\n\n> I see in the code that it says that the API is experimental. Is\n> there any absolute need for backward compatibility, or can I try to\n> redesign the API somewhat extensively?\n\nBeing experimental merely means that we do not support out of tree\nusers; it does not mean you are allowed to break it in any way for the\nin-tree users.\n"},{"id":"190143","messageId":"4F99A91F.3050307@vilain.net","threadId":"30337","inReplyTo":"CAB3zAY3-Bn86bCr7Rxqi4vxbYFxUesLwm8gddxyMSexov2tOhw@mail.gmail.com","subject":"Re: Git.pm","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2012-04-26T19:59:27Z","receivedAt":"2012-04-26T19:59:27Z","isPatch":false,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 4/25/12 9:15 PM, Subho Banerjee wrote:\n> ---> I see in the code that it says that the API is experimental. Is\n> there any absolute need for backward compatibility, or can I try to\n> redesign the API somewhat extensively?\n\nIf you stick to putting new APIs under different namespaces, or new \nfunctions, then you should be able to preserve API compatibility. I \nthink Git.pm is now too widely used for breaking compatibility to be an \noption.\n\nI think I submitted a Git::Config to this list some time ago; did you \nfind that?\n\nSam\n"},{"id":"190144","messageId":"CAB3zAY0NeXuH-wXyYkbim5U74eANY4hq5D6SsVLu3KeUqHFqzQ@mail.gmail.com","threadId":"30337","inReplyTo":"CAFouetgwRpB1GFJOC8PTVryVY-94S3xa5ZiSaWQWoz070qQ-6g@mail.gmail.com","subject":"Re: Git.pm","fromName":"Subho Banerjee","fromEmail":"subs.zero@gmail.com","sentAt":"2012-04-26T20:10:07Z","receivedAt":"2012-04-26T20:10:07Z","isPatch":false,"sender":{"key":"subs.zero@gmail.com","avatar":"https://gravatar.com/avatar/6159910e05d7650fc7da0a77c233711a014ea449cc5b30f0f065d0017b3357dd?d=mp&s=160"},"body":"Hello,\nI will take care that I dont break those. Should the tests in the t/\nfolder of the codebase be enough to make sure everything is working as\nit should be even in the Git perl module? Also is there anything like\na public build server which actually catalogs which tests are\ncurrently failing so that I know what has gone wrong after my changes,\nor are all commits supposed to pass every test?\n\nCheers,\nSubho.\n\nOn Fri, Apr 27, 2012 at 12:28 AM, Tim Henigan wrote:\n> On Thu, Apr 26, 2012 at 12:15 AM, Subho Banerjee wrote:\n>>\n>> ---> I see in the code that it says that the API is experimental. Is\n>> there any absolute need for backward compatibility, or can I try to\n>> redesign the API somewhat extensively?\n>\n> A quick grep of the code in 'master' shows Git.pm used in the following:\n>\n>    - contrib/examples/git-remote.perl\n>    - git-add--interactive.perl\n>    - git-cvsexportcommit.perl\n>    - git-send-email.perl\n>    - git-svn.perl\n>    - t/perf/aggregate.perl\n>\n> There is also work in progress on 'pu' that relies on Git.pm.\n>\n> Breaking any of these scripts would be bad.  You may be able to\n> refactor them at the same time Git.pm is modified, but it would be\n> wise to contact the authors before making any major changes.\n"},{"id":"190152","messageId":"20120426203136.GA15432@burratino","threadId":"30337","inReplyTo":"CAB3zAY0NeXuH-wXyYkbim5U74eANY4hq5D6SsVLu3KeUqHFqzQ@mail.gmail.com","subject":"Re: Git.pm","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-26T20:31:36Z","receivedAt":"2012-04-26T20:31:36Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nSubho Banerjee wrote:\n\n> I will take care that I dont break those. \n\nThanks, sounds good.\n\n> Should the tests in the t/\n> folder of the codebase be enough to make sure everything is working as\n> it should be even in the Git perl module?\n\nNo. :)\n\n> Also is there anything like\n> a public build server which actually catalogs which tests are\n> currently failing so that I know what has gone wrong after my changes,\n> or are all commits supposed to pass every test?\n\nWhen tests are known to fail, they are marked with test_expect_failure\nso they don't affect the test result.\n\nHope that helps,\nJonathan\n"},{"id":"191284","messageId":"CAB3zAY3VHtUobJfJ7=nSKb_6uJOXLGVHzR18qV6txPkzf54cDw@mail.gmail.com","threadId":"30337","inReplyTo":"20120426203136.GA15432@burratino","subject":"Re: Git.pm","fromName":"Subho Banerjee","fromEmail":"subs.zero@gmail.com","sentAt":"2012-05-10T13:19:36Z","receivedAt":"2012-05-10T13:19:36Z","isPatch":false,"sender":{"key":"subs.zero@gmail.com","avatar":"https://gravatar.com/avatar/6159910e05d7650fc7da0a77c233711a014ea449cc5b30f0f065d0017b3357dd?d=mp&s=160"},"body":"Hello,\nI have started looking into how the error catching mechanism\nimplemented right now. I have looked into the more modern error\ncatching/throwing mechanisms in use in perl, and I am of the opinion\nthat Try::Simple would probably be the best candidate for being the\nnew error catching mechanism. I also wanted to discuss some aspects of\nthe changes to be made -\n------- Replacing the Error::Simple stuff should be relatively\nstraightforward. It can be achieved with simple changes to the syntax\nof the perl module itself.\n\n------- What I feel will be more complicated, and will require some\ndiscussion before it is implemented is the Git::Error module. This has\nmodified some of the code in the original Error module and is used\nonly when there are calls made to the git system command. Using the\nTry::Tiny will mean that this can be simplfied to a very large extent.\nAs a mater of fact I am in favor of getting rid of this completely and\nimplementing whatever is required in the Git.pm as required. Because\nthe Try::Tiny module no longer requires exception objects to be\nthrown. Its just simply passing strings around.\n\nThis I believe is a big decision, and I would like to hear what you\nguys have to say before I actually get along changing and playing\naround with stuff inside the code.\n\nCheers,\nSubho.\n\nOn Fri, Apr 27, 2012 at 2:01 AM, Jonathan Nieder wrote:\n> Hi,\n>\n> Subho Banerjee wrote:\n>\n>> I will take care that I dont break those.\n>\n> Thanks, sounds good.\n>\n>>                                           Should the tests in the t/\n>> folder of the codebase be enough to make sure everything is working as\n>> it should be even in the Git perl module?\n>\n> No. :)\n>\n>>                                           Also is there anything like\n>> a public build server which actually catalogs which tests are\n>> currently failing so that I know what has gone wrong after my changes,\n>> or are all commits supposed to pass every test?\n>\n> When tests are known to fail, they are marked with test_expect_failure\n> so they don't affect the test result.\n>\n> Hope that helps,\n> Jonathan\n"},{"id":"191303","messageId":"20120510151650.GA6591@burratino","threadId":"30337","inReplyTo":"CAB3zAY3VHtUobJfJ7=nSKb_6uJOXLGVHzR18qV6txPkzf54cDw@mail.gmail.com","subject":"Re: Git.pm","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-05-10T15:16:50Z","receivedAt":"2012-05-10T15:16:50Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Subho,\n\nSubho Banerjee wrote:\n\n> I have started looking into how the error catching mechanism\n> implemented right now. I have looked into the more modern error\n> catching/throwing mechanisms in use in perl, and\n[...]\n> This I believe is a big decision, and I would like to hear what you\n> guys have to say before I actually get along changing and playing\n> around with stuff inside the code.\n\nI'm cc-ing Jakub in case he has thoughts on this. Otherwise, my\nsuggestion would be to make a small trial change and send out a\npatch to get feedback. It's not a big decision until we actually\napply the patch. :)\n\nHope that helps,\nJonathan\n"},{"id":"191312","messageId":"CANgJU+W-FJZRtu_4si7nr96KfNe2rzaiUaDC0GiK_WixudvcxA@mail.gmail.com","threadId":"30337","inReplyTo":"CAB3zAY3VHtUobJfJ7=nSKb_6uJOXLGVHzR18qV6txPkzf54cDw@mail.gmail.com","subject":"Re: Git.pm","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2012-05-10T15:54:38Z","receivedAt":"2012-05-10T15:54:38Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 10 May 2012 15:19, Subho Banerjee wrote:\n> Hello,\n> I have started looking into how the error catching mechanism\n> implemented right now. I have looked into the more modern error\n> catching/throwing mechanisms in use in perl, and I am of the opinion\n> that Try::Simple would probably be the best candidate for being the\n> new error catching mechanism. I also wanted to discuss some aspects of\n> the changes to be made -\n> ------- Replacing the Error::Simple stuff should be relatively\n> straightforward. It can be achieved with simple changes to the syntax\n> of the perl module itself.\n>\n> ------- What I feel will be more complicated, and will require some\n> discussion before it is implemented is the Git::Error module. This has\n> modified some of the code in the original Error module and is used\n> only when there are calls made to the git system command. Using the\n> Try::Tiny will mean that this can be simplfied to a very large extent.\n> As a mater of fact I am in favor of getting rid of this completely and\n> implementing whatever is required in the Git.pm as required. Because\n> the Try::Tiny module no longer requires exception objects to be\n> thrown. Its just simply passing strings around.\n>\n> This I believe is a big decision, and I would like to hear what you\n> guys have to say before I actually get along changing and playing\n> around with stuff inside the code.\n\nPersonally I would prefer it just does error handling like any other\nstandard Perl code does: either return false, or dies with a useful\nerror message. One of the things I find annoying about Git.pm is it\nforces its authors non-standard preferences for exception handling\nonto its users.\n\nAny other approach forces people to use the exception framework you\nhave chosen. Which is just a pain in the ass.\n\nSimilar logic for Try::Tiny. Why bother with it? It is pretty close to\na fancy way to write eval { ...; 1 } or do { .... }; It is just one\nmore module for people to misunderstand, and then make bugs with.\n\nWhy require people coding on your module to learn a new way to eval code?\n\nYes I know in some circles these are probably controversial points,\nbut in all the core, heavily used Perl code I know of none of it uses\neither exception objects nor Try::Tiny. I think there is a reason why.\n\nSo think carefully. Look at DBI.pm for guidance. That module is\nprobably the single most stable, well maintained and widely used\nmodule in Perl. And it does none of the tricks you discuss here.\n\nYves\n\n\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"191313","messageId":"CAB3zAY0NAQaN-nNeJdJy80omrXqUZ-vCWsFhbx_iHF5RPBYUQQ@mail.gmail.com","threadId":"30337","inReplyTo":"CANgJU+W-FJZRtu_4si7nr96KfNe2rzaiUaDC0GiK_WixudvcxA@mail.gmail.com","subject":"Re: Git.pm","fromName":"Subho Banerjee","fromEmail":"subs.zero@gmail.com","sentAt":"2012-05-10T16:18:28Z","receivedAt":"2012-05-10T16:18:28Z","isPatch":false,"sender":{"key":"subs.zero@gmail.com","avatar":"https://gravatar.com/avatar/6159910e05d7650fc7da0a77c233711a014ea449cc5b30f0f065d0017b3357dd?d=mp&s=160"},"body":"Hello Yves,\nI am aware of that. But you see the problem is that using eval/warn/do\nis that the $@ has to be localized every time eval is called. From my\nunderstanding of how the Try::Tiny package works, this is exactly what\nhappens. So we are just calling a simple eval statement but in a block\nwhere the $@ is handled properly, which is eventually what we would\nhave to do if we wrote it ourselves(Though I am not sure about how DBI\ndoes it, I will have a look into that). And that is why I arrived in\nfavor of the Try::Tiny module in the first place. Well, that and the\nability to throw exception objects if required.\n\nCheers,\nSubho.\n\nOn Thu, May 10, 2012 at 9:24 PM, demerphq wrote:\n> On 10 May 2012 15:19, Subho Banerjee wrote:\n>> Hello,\n>> I have started looking into how the error catching mechanism\n>> implemented right now. I have looked into the more modern error\n>> catching/throwing mechanisms in use in perl, and I am of the opinion\n>> that Try::Simple would probably be the best candidate for being the\n>> new error catching mechanism. I also wanted to discuss some aspects of\n>> the changes to be made -\n>> ------- Replacing the Error::Simple stuff should be relatively\n>> straightforward. It can be achieved with simple changes to the syntax\n>> of the perl module itself.\n>>\n>> ------- What I feel will be more complicated, and will require some\n>> discussion before it is implemented is the Git::Error module. This has\n>> modified some of the code in the original Error module and is used\n>> only when there are calls made to the git system command. Using the\n>> Try::Tiny will mean that this can be simplfied to a very large extent.\n>> As a mater of fact I am in favor of getting rid of this completely and\n>> implementing whatever is required in the Git.pm as required. Because\n>> the Try::Tiny module no longer requires exception objects to be\n>> thrown. Its just simply passing strings around.\n>>\n>> This I believe is a big decision, and I would like to hear what you\n>> guys have to say before I actually get along changing and playing\n>> around with stuff inside the code.\n>\n> Personally I would prefer it just does error handling like any other\n> standard Perl code does: either return false, or dies with a useful\n> error message. One of the things I find annoying about Git.pm is it\n> forces its authors non-standard preferences for exception handling\n> onto its users.\n>\n> Any other approach forces people to use the exception framework you\n> have chosen. Which is just a pain in the ass.\n>\n> Similar logic for Try::Tiny. Why bother with it? It is pretty close to\n> a fancy way to write eval { ...; 1 } or do { .... };  It is just one\n> more module for people to misunderstand, and then make bugs with.\n>\n> Why require people coding on your module to learn a new way to eval code?\n>\n> Yes I know in some circles these are probably controversial points,\n> but in all the core, heavily used Perl code I know of none of it uses\n> either exception objects nor Try::Tiny. I think there is a reason why.\n>\n> So think carefully. Look at DBI.pm for guidance. That module is\n> probably the single most stable, well maintained and widely used\n> module in Perl. And it does none of the tricks you discuss here.\n>\n> Yves\n>\n>\n>\n>\n> --\n> perl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"191314","messageId":"7vr4usnh2q.fsf@alter.siamese.dyndns.org","threadId":"30337","inReplyTo":"CANgJU+W-FJZRtu_4si7nr96KfNe2rzaiUaDC0GiK_WixudvcxA@mail.gmail.com","subject":"Re: Git.pm","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T16:20:29Z","receivedAt":"2012-05-10T16:20:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"demerphq writes:\n\n> Similar logic for Try::Tiny. Why bother with it? It is pretty close to\n> a fancy way to write eval { ...; 1 } or do { .... }; It is just one\n> more module for people to misunderstand, and then make bugs with.\n\nI personally like the approach to stick to bare \"eval {}; if ($@) { ... }\"\nsequence, as it is much more explicit and easier to understand what is\nhappening underneath. IOW, I like what I read in demerphq's message.\n\nBut it could be that these many people who wrote these different\ncatch/throw things did so for a reason that I am missing, and if that is\nthe case, I am interested to hear what benefit we will get from using\nthem.\n\n\"It looks more familiar to people with (your favorite language)\" could be\nit, but then I would not regret missing such a reason ;-)\n\nThanks.\n"},{"id":"191328","messageId":"CANgJU+XFDMETTKxEeBO01d=qwoD+xjP51A6UKEub_t1AFU625A@mail.gmail.com","threadId":"30337","inReplyTo":"CAB3zAY0NAQaN-nNeJdJy80omrXqUZ-vCWsFhbx_iHF5RPBYUQQ@mail.gmail.com","subject":"Re: Git.pm","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2012-05-10T17:22:59Z","receivedAt":"2012-05-10T17:22:59Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 10 May 2012 18:18, Subho Banerjee wrote:\n> Hello Yves,\n> I am aware of that. But you see the problem is that using eval/warn/do\n> is that the $@ has to be localized every time eval is called.\n\nIt doesn't *have* to be localized every time. Although it does help a\nbit. On the other hand it also hinders.\n\n> From my\n> understanding of how the Try::Tiny package works, this is exactly what\n> happens. So we are just calling a simple eval statement but in a block\n> where the $@ is handled properly, which is eventually what we would\n> have to do if we wrote it ourselves(Though I am not sure about how DBI\n> does it, I will have a look into that). And that is why I arrived in\n> favor of the Try::Tiny module in the first place. Well, that and the\n> ability to throw exception objects if required.\n\nAs far as I know Try::Tiny exists because of a bug in Perl.\n\nThe bug is that since $@ is a global, if an object is created in an\neval, and then its DESTROY itself does an eval it can \"hide\" the\nerror, if one uses eval incorrectly. However if one does things\ncorrectly one can detect this case regardless, although perhaps in the\nprocess losing the error message. Ny simply arranging that every eval\nstatements returns a true value then you can ALWAYS detect when an\neval failed, even if $@ is clobbered.\n\n$ perl -le'sub Foo::DESTROY { print \"in DESTROY\"; eval 1 } my $ok=\neval q(my $o= bless {}, \"Foo\"; my $zero=0; print 1/$zero; 1); if ($@)\n{ print $@ } if (!$ok) { print \"error in eval: \", $@||\"Zombie Error\"}'\nin DESTROY\nerror in eval: Zombie Error\n\nremove the eval 1 in the DESTROY method:\n\n$ perl -le'sub Foo::DESTROY { print \"in DESTROY\"; } my $ok= eval q(my\n$o= bless {}, \"Foo\"; my $zero=0; print 1/$zero; 1); if ($@) { print $@\n} if (!$ok) { print \"error in eval: \", $@||\"Zombie Error\"}'\nin DESTROY\nIllegal division by zero at (eval 1) line 1.\n\nerror in eval: Illegal division by zero at (eval 1) line 1.\n\nNow it is true that using local does somewhat save things, however\nONLY if EVERYONE uses local when they eval. Which they dont. Nor do\nthey use Try::Tiny. So in regards to localization the only thing\nTry::Tiny saves you from is other people using Try::Tiny or similar\nfunctionality.\n\nNow, it is true that Try::Tiny arranges to check the eval, and it\nlocalizes $@, so I suppose it does save some people from shooting\nthemselves in the foot. But Perl is most definitely not about not\nshooting yourself in the foot, indeed, if you ask it nicely Perl will\nhand you a loaded shotgun to make it easier.\n\nBasically all Try::Tiny does is make:\n\ntry {\n do_something();\n} catch {\n do_something_with_error($_);\n};\n\nbehave the same as this:\n\nlocal $@;\neval {\n do_something();\n 1;\n} or do {\n my $error= $@ || \"Zombie Error\";\n do_something_with_error($error);\n};\n\nWhich for me is silly. Id rather see the code than have a module wrap\nit up in sub calls, replace $@ with $_ and related junk. Yes this\nrequires Perl programmers to know their stuff. But then so does any\nnon-trivial programming task.\n\nNow notice some issues with Try::Tiny, it doesnt support string eval\nanyway, so you can just swap it into the code I wrote, nor the general\ncase of eval. And even if you use it, it doesnt save you if the code\nyou are executing does not ALSO use it:\n\n$ perl -MTry::Tiny -le'sub Foo::DESTROY { print \"in DESTROY\"; eval 1 }\nmy $ok= eval {my $o= bless {}, \"Foo\"; my $zero=0; print 1/$zero; 1};\nif ($@) { print $@ } if (!$ok) { print \"error in eval: \", $@||\"Zombie\nError\"}'\nin DESTROY\nerror in eval: Zombie Error\n$ perl -MTry::Tiny -le'sub Foo::DESTROY { print \"in DESTROY\"; eval 1 }\nmy $ok= try {my $o= bless {}, \"Foo\"; my $zero=0; print 1/$zero; 1}; if\n($@) { print $@ } if (!$ok) { print \"error in eval: \", $@||\"Zombie\nError\"}'\nin DESTROY\nerror in eval: Zombie Error\n$ perl -MTry::Tiny -le'sub Foo::DESTROY { print \"in DESTROY\"; eval 1 }\nmy $ok= try {my $o= bless {}, \"Foo\"; my $zero=0; print 1/$zero; 1}\ncatch { print \"error in eval: \", $_||\"Zombie Error\"}'\nin DESTROY\nerror in eval: Zombie Error\n\nNotice how it provides no more benefit than writing eval like this:\n\neval {\n stuff();\n 1; # this is important\n} or do {\n my $error= $@ || \"zombie error\"; # must do this as early as possible\n do_something_with_error($error);\n};\n\nAnd for me, if I see code written like this I KNOW it works, i know\nwhat side effects it has (localizing $@ has its own issues), and I\nknow I can understand it.\n\nSo for me Try::Tiny is a waste of time. Using it saves you from almost\nnothing, and just forces consumers of your code to know how to do eval\nproperly AND how Try::Tiny works. Not using Try::Tiny at all means\npeople JUST have to know how to use eval properly, which if they want\nto do any kind of real Perl work they need to know anyway.\n\nAnyway, on the side of annoying you about Perl trivia, you can find a\nlot of reusable library code in git-deploy on github. Feel free to\nsteal whatever you like.\n\n https://github.com/git-deploy\n\nI would have loved to have a better git library module than Git.pm\nwhen I wrote the original version of git-deploy. So I am all in favour\nof your project.\n\ncheers,\nYves\n\n\n\n\n\n\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"191330","messageId":"CANgJU+WR9zWbwrHK-PT0jKKNQ6ZXv=9oGxOuQh6iLZaORohGBQ@mail.gmail.com","threadId":"30337","inReplyTo":"7vr4usnh2q.fsf@alter.siamese.dyndns.org","subject":"Re: Git.pm","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2012-05-10T17:38:29Z","receivedAt":"2012-05-10T17:38:29Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 10 May 2012 18:20, Junio C Hamano wrote:\n> demerphq writes:\n>\n>> Similar logic for Try::Tiny. Why bother with it? It is pretty close to\n>> a fancy way to write eval { ...; 1 } or do { .... };  It is just one\n>> more module for people to misunderstand, and then make bugs with.\n>\n> I personally like the approach to stick to bare \"eval {}; if ($@) { ... }\"\n> sequence, as it is much more explicit and easier to understand what is\n> happening underneath.  IOW, I like what I read in demerphq's message.\n\nBasically that is the idiom that Try::Tiny encourages people not to\nuse. Unfortunately the idiom documented in the Perl docs has been\nsubtly wrong for pretty much ever due to a subtle bug in perl (which\ntook years to come to light). :-(\n\nAnyway, anything written like this:\n\neval {\n whatever();\n 1;\n} or do {\n my $error= $@ || \"Zombie Error\";\n do_something_with_error($error);\n};\n\nis fine. See my other post to the list for details.\n\n> But it could be that these many people who wrote these different\n> catch/throw things did so for a reason that I am missing, and if that is\n> the case, I am interested to hear what benefit we will get from using\n> them.\n>\n> \"It looks more familiar to people with (your favorite language)\" could be\n> it, but then I would not regret missing such a reason ;-)\n\nA few might justify things based on the bug in Perl, but I suspect the\nreal motivation people have to use it is to emulate other languages\nconstructs.\n\nThe author of Try::Tiny might be an exception, I am reasonably\nconvinced he was trying to provide a service to the community, but IMO\non the balance of things the module muddies the water more than it\nimproves things.\n\ncheers,\nYves\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"191355","messageId":"4FAC2B2E.7060101@pileofstuff.org","threadId":"30337","inReplyTo":"CAB3zAY3VHtUobJfJ7=nSKb_6uJOXLGVHzR18qV6txPkzf54cDw@mail.gmail.com","subject":"Re: Git.pm","fromName":"Andrew Sayers","fromEmail":"andrew-git@pileofstuff.org","sentAt":"2012-05-10T20:55:10Z","receivedAt":"2012-05-10T20:55:10Z","isPatch":false,"sender":{"key":"andrew-git@pileofstuff.org","avatar":null},"body":"Try::Tiny is an increasingly standard part of Perl - for example, it's\nused extensively in Moose. There's a good list of arguments about why\nyou should use it instead of eval in the Try::Tiny documentation:\nhttp://search.cpan.org/~nuffin/Try-Tiny-0.01/lib/Try/Tiny.pm\n\nNow I've got that talking point done, here's what I really think :)\n\nTry::Tiny is designed on the assumption that throwing and catching\nobjects is something people should do all the time, and it can cause\nsubtle errors that are only worth the hassle if you get a lot of benefit\nfrom doing so. It's easy enough to come up with ideas for where they\nmight be useful, but in the real world advanced uses for exceptions are\nusually a sign you're doing it wrong. Three of the most common reasons\nfor frequent/complex exceptions are handling errors further up the call\nstack, recovering from operations that fail, and clever error-handling.\n\n\nIf you want exceptions to be caught by code further up the call stack\nthan the immediate caller, you're likely to be disappointed. This is\none of the places where \"separation of concerns\" applies - if I use a\nmodule that uses a module that uses your module, then catching\nexceptions from your code will just cause my program to break when some\nmodule in the middle obscures your error by adding its own layer of\nerror handling.\n\n\nIf you have an operation that really might fail, and you want to\nencourage most people to handle it most of the time, it's better to have\na function with a meaningful name and good documentation. This puts the\nburden on the calling function to handle the error instead of letting\nthem think \"oh well, if it dies someone else will handle it\". It also\nforces you to split functions along boundaries that make your code\nreadable, instead of falling for the temptation to make something that\n\"just works\"... until it doesn't, and the maintainer has to go\nspelunking through code they don't know. So instead of:\n\n try {\n Foo::frobnicate( widgets => 3 );\n } catch {\n if (ref($_) eq 'Error::Widget') {\n die \"Could not add 3 widgets\";\n }\n }\n\nIt's better to ask the people using your module to write:\n\n my $foo = Foo->new;\n $foo->add_widgets(3) or die \"could not add 3 widgets\"\n $foo->frobnicate;\n\nThis is easier to document, easier to write and easier to read.\n\n\nIf you have an operation where calling code is supposed to do something\nmore complicated than give up, it's better to use a callback. This\ngives you an opportunity to document what's needed, and to check that\nthe calling code is doing the right thing before it's too late. So\ninstead of:\n\n my $widgets = 3;\n while ( $widgets ) {\n\n try {\n Foo::frobnicate($widgets);\n $widgets = 0;\n } catch {\n if ( $_->{remaining_widgets} < 2 ) {\n die $_->{error};\n } elsif ( $_->{remaining_widgets} == 2 ) {\n $widgets = 0;\n }\n }\n\n }\n\nIt's better to ask people using your module to write:\n\n Foo::Frobnicate(\n widgets => 3,\n error_handler => sub {\n my ( $remaining_widgets, $error ) = @_;\n die $error if $broken_widgets < 2;\n return \"give up\" if $remaining_widgets == 2;\n return \"continue\";\n },\n );\n\nAgain, this is more readable and easier to document.\n\n\nAside from the philosophical angle, Try::Tiny is particularly hard to\nmaintain because it looks like a language extension, but is actually\njust an ordinary module. The try {} and catch {} blocks are anonymous\nsubroutines, which lead to some wonderfully unintuitive behaviour. See\nwhat you think these do, then run the code to find out:\n\n\n sub foo {\n try {\n return 1;\n }\n return 0;\n }\n\n sub bar {\n\n our @args = @_;\n our @ret;\n\n try {\n @ret =\n wantarray\n ? grep( /blah/, @args )\n : [ grep( /blah/, @args ) ]\n };\n\n return @ret;\n }\n\n my $foo = \"bar\";\n sub baz {\n my $foo = \"baz\";\n try {\n print $foo;\n }\n }\n\n sub qux {\n my $ret;\n try {\n $ret = \"value\";\n }\n return $ret;\n }\n\n print foo, \"\\n\";\n print bar( \"blah\", \"blip\" ), \"\\n\";\n baz;\n print qux, \"\\n\";\n\n\nIn short, Try::Tiny looks like a lot of gain for not much pain, but\nactually it's the other way around.\n\n\t- Andrew\n"},{"id":"191382","messageId":"CANgJU+XuPo00U+7r1xtK0BcD1YfrWhpCZ2DO10DEc_PpOvtFdQ@mail.gmail.com","threadId":"30337","inReplyTo":"4FAC2B2E.7060101@pileofstuff.org","subject":"Re: Git.pm","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2012-05-11T08:27:31Z","receivedAt":"2012-05-11T08:27:31Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 10 May 2012 22:55, Andrew Sayers wrote:\n> Try::Tiny is an increasingly standard part of Perl - for example, it's\n> used extensively in Moose.  There's a good list of arguments about why\n> you should use it instead of eval in the Try::Tiny documentation:\n> http://search.cpan.org/~nuffin/Try-Tiny-0.01/lib/Try/Tiny.pm\n>\n> Now I've got that talking point done, here's what I really think :)\n>\n> Try::Tiny is designed on the assumption that throwing and catching\n> objects is something people should do all the time, and it can cause\n> subtle errors that are only worth the hassle if you get a lot of benefit\n> from doing so.  It's easy enough to come up with ideas for where they\n> might be useful, but in the real world advanced uses for exceptions are\n> usually a sign you're doing it wrong.  Three of the most common reasons\n> for frequent/complex exceptions are handling errors further up the call\n> stack, recovering from operations that fail, and clever error-handling.\n>\n>\n> If you want exceptions to be caught by code further up the call stack\n> than the immediate caller, you're likely to be disappointed.  This is\n> one of the places where \"separation of concerns\" applies - if I use a\n> module that uses a module that uses your module, then catching\n> exceptions from your code will just cause my program to break when some\n> module in the middle obscures your error by adding its own layer of\n> error handling.\n>\n>\n> If you have an operation that really might fail, and you want to\n> encourage most people to handle it most of the time, it's better to have\n> a function with a meaningful name and good documentation.  This puts the\n> burden on the calling function to handle the error instead of letting\n> them think \"oh well, if it dies someone else will handle it\".  It also\n> forces you to split functions along boundaries that make your code\n> readable, instead of falling for the temptation to make something that\n> \"just works\"... until it doesn't, and the maintainer has to go\n> spelunking through code they don't know.  So instead of:\n>\n>   try {\n>       Foo::frobnicate( widgets => 3 );\n>   } catch {\n>      if (ref($_) eq 'Error::Widget') {\n>          die \"Could not add 3 widgets\";\n>      }\n>   }\n>\n> It's better to ask the people using your module to write:\n>\n>   my $foo = Foo->new;\n>   $foo->add_widgets(3) or die \"could not add 3 widgets\"\n>   $foo->frobnicate;\n>\n> This is easier to document, easier to write and easier to read.\n>\n>\n> If you have an operation where calling code is supposed to do something\n> more complicated than give up, it's better to use a callback.  This\n> gives you an opportunity to document what's needed, and to check that\n> the calling code is doing the right thing before it's too late.  So\n> instead of:\n>\n>    my $widgets = 3;\n>    while ( $widgets ) {\n>\n>        try {\n>            Foo::frobnicate($widgets);\n>            $widgets = 0;\n>        } catch {\n>            if ( $_->{remaining_widgets} < 2 ) {\n>                die $_->{error};\n>            } elsif ( $_->{remaining_widgets} == 2 ) {\n>                $widgets = 0;\n>            }\n>        }\n>\n>    }\n>\n> It's better to ask people using your module to write:\n>\n>    Foo::Frobnicate(\n>        widgets => 3,\n>        error_handler => sub {\n>             my ( $remaining_widgets, $error ) = @_;\n>             die $error if $broken_widgets < 2;\n>             return \"give up\" if $remaining_widgets == 2;\n>             return \"continue\";\n>        },\n>    );\n>\n> Again, this is more readable and easier to document.\n>\n>\n> Aside from the philosophical angle, Try::Tiny is particularly hard to\n> maintain because it looks like a language extension, but is actually\n> just an ordinary module.  The try {} and catch {} blocks are anonymous\n> subroutines, which lead to some wonderfully unintuitive behaviour.  See\n> what you think these do, then run the code to find out:\n>\n>\n>    sub foo {\n>        try {\n>            return 1;\n>        }\n>        return 0;\n>    }\n>\n>    sub bar {\n>\n>        our @args = @_;\n>        our @ret;\n>\n>        try {\n>            @ret =\n>                wantarray\n>                    ?   grep( /blah/, @args )\n>                    : [ grep( /blah/, @args ) ]\n>        };\n>\n>        return @ret;\n>    }\n>\n>    my $foo = \"bar\";\n>    sub baz {\n>        my $foo = \"baz\";\n>        try {\n>            print $foo;\n>        }\n>    }\n>\n>    sub qux {\n>        my $ret;\n>        try {\n>            $ret = \"value\";\n>        }\n>        return $ret;\n>    }\n>\n>    print foo, \"\\n\";\n>    print bar( \"blah\", \"blip\" ), \"\\n\";\n>    baz;\n>    print qux, \"\\n\";\n>\n>\n> In short, Try::Tiny looks like a lot of gain for not much pain, but\n> actually it's the other way around.\n\nTotal agreement.\n\nYves\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"191399","messageId":"86likyy7ub.fsf@red.stonehenge.com","threadId":"30337","inReplyTo":"CAB3zAY3VHtUobJfJ7=nSKb_6uJOXLGVHzR18qV6txPkzf54cDw@mail.gmail.com","subject":"Re: Git.pm","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2012-05-11T16:56:44Z","receivedAt":"2012-05-11T16:56:44Z","isPatch":false,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Subho\" == Subho Banerjee writes:\n\nSubho> I have started looking into how the error catching mechanism\nSubho> implemented right now. I have looked into the more modern error\nSubho> catching/throwing mechanisms in use in perl, and I am of the opinion\nSubho> that Try::Simple would probably be the best candidate for being the\nSubho> new error catching mechanism. I also wanted to discuss some aspects of\nSubho> the changes to be made -\n\nTry::Tiny is preferred to Try::Simple.\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n \nSmalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.\nSee http://methodsandmessages.posterous.com/ for Smalltalk discussion\n"},{"id":"191406","messageId":"7vobpulhbk.fsf@alter.siamese.dyndns.org","threadId":"30337","inReplyTo":"86likyy7ub.fsf@red.stonehenge.com","subject":"Re: Git.pm","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-11T18:10:23Z","receivedAt":"2012-05-11T18:10:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"merlyn@stonehenge.com (Randal L. Schwartz) writes:\n\n>>>>>> \"Subho\" == Subho Banerjee writes:\n>\n> Subho> I have started looking into how the error catching mechanism\n> Subho> implemented right now. I have looked into the more modern error\n> Subho> catching/throwing mechanisms in use in perl, and I am of the opinion\n> Subho> that Try::Simple would probably be the best candidate for being the\n> Subho> new error catching mechanism. I also wanted to discuss some aspects of\n> Subho> the changes to be made -\n>\n> Try::Tiny is preferred to Try::Simple.\n\nThanks for an expert input; could you give another comparison between\nTry::Tiny vs the use of bare \"eval / if ($@)\"?\n"},{"id":"191725","messageId":"1337411317-14931-1-git-send-email-subs.zero@gmail.com","threadId":"30337","inReplyTo":"7vobpulhbk.fsf@alter.siamese.dyndns.org","subject":"[PATCH][GIT.PM 1/3] Ignore files produced from exuberant-ctags","fromName":"Subho Sankar Banerjee","fromEmail":"subs.zero@gmail.com","sentAt":"2012-05-19T07:08:35Z","receivedAt":"2012-05-19T07:08:35Z","isPatch":true,"sender":{"key":"subs.zero@gmail.com","avatar":"https://gravatar.com/avatar/6159910e05d7650fc7da0a77c233711a014ea449cc5b30f0f065d0017b3357dd?d=mp&s=160"},"body":"\nSigned-off-by: Subho Sankar Banerjee \n---\n .gitignore | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/.gitignore b/.gitignore\nindex fb7d559..8ecd336 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -219,6 +219,7 @@\n /configure\n /tags\n /TAGS\n+tags\n /cscope*\n *.obj\n *.lib\n-- \n1.7.9.5\n"},{"id":"191726","messageId":"1337411317-14931-2-git-send-email-subs.zero@gmail.com","threadId":"30337","inReplyTo":"1337411317-14931-1-git-send-email-subs.zero@gmail.com","subject":"[PATCH][GIT.PM 2/3] Getting rid of throwing Error::Simple objects in favour of simple Perl scalars which can be caught in eval{} blocks","fromName":"Subho Sankar Banerjee","fromEmail":"subs.zero@gmail.com","sentAt":"2012-05-19T07:08:36Z","receivedAt":"2012-05-19T07:08:36Z","isPatch":true,"sender":{"key":"subs.zero@gmail.com","avatar":"https://gravatar.com/avatar/6159910e05d7650fc7da0a77c233711a014ea449cc5b30f0f065d0017b3357dd?d=mp&s=160"},"body":"\nSigned-off-by: Subho Sankar Banerjee \n---\n perl/Git.pm | 52 ++++++++++++++++++++++++++--------------------------\n 1 file changed, 26 insertions(+), 26 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 497f420..52777d4 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -160,7 +160,7 @@ sub repository {\n \tif (defined $args[0]) {\n \t\tif ($#args % 2 != 1) {\n \t\t\t# Not a hash.\n-\t\t\t$#args == 0 or throw Error::Simple(\"bad usage\");\n+\t\t\t$#args == 0 or die \"bad usage\";\n \t\t\t%opts = ( Directory => $args[0] );\n \t\t} else {\n \t\t\t%opts = @args;\n@@ -173,7 +173,7 @@ sub repository {\n \t}\n \n \tif (defined $opts{Directory}) {\n-\t\t-d $opts{Directory} or throw Error::Simple(\"Directory not found: $opts{Directory} $!\");\n+\t\t-d $opts{Directory} or die \"Directory not found: $opts{Directory} $!\";\n \n \t\tmy $search = Git->repository(WorkingCopy => $opts{Directory});\n \t\tmy $dir;\n@@ -193,7 +193,7 @@ sub repository {\n \t\t\t$dir = abs_path($opts{Directory}) . '/';\n \t\t\tif ($prefix) {\n \t\t\t\tif (substr($dir, -length($prefix)) ne $prefix) {\n-\t\t\t\t\tthrow Error::Simple(\"rev-parse confused me - $dir does not have trailing $prefix\");\n+\t\t\t\t\tdie \"rev-parse confused me - $dir does not have trailing $prefix\";\n \t\t\t\t}\n \t\t\t\tsubstr($dir, -length($prefix)) = '';\n \t\t\t}\n@@ -206,14 +206,14 @@ sub repository {\n \n \t\t\tunless (-d \"$dir/refs\" and -d \"$dir/objects\" and -e \"$dir/HEAD\") {\n \t\t\t\t# Mimic git-rev-parse --git-dir error message:\n-\t\t\t\tthrow Error::Simple(\"fatal: Not a git repository: $dir\");\n+\t\t\t\tdie \"fatal: Not a git repository: $dir\";\n \t\t\t}\n \t\t\tmy $search = Git->repository(Repository => $dir);\n \t\t\ttry {\n \t\t\t\t$search->command('symbolic-ref', 'HEAD');\n \t\t\t} catch Git::Error::Command with {\n \t\t\t\t# Mimic git-rev-parse --git-dir error message:\n-\t\t\t\tthrow Error::Simple(\"fatal: Not a git repository: $dir\");\n+\t\t\t\tdie \"fatal: Not a git repository: $dir\";\n \t\t\t}\n \n \t\t\t$opts{Repository} = abs_path($dir);\n@@ -469,7 +469,7 @@ sub command_noisy {\n \n \tmy $pid = fork;\n \tif (not defined $pid) {\n-\t\tthrow Error::Simple(\"fork failed: $!\");\n+\t\tdie \"fork failed: $!\";\n \t} elsif ($pid == 0) {\n \t\t_cmd_exec($self, $cmd, @args);\n \t}\n@@ -552,10 +552,10 @@ and the directory must exist.\n sub wc_chdir {\n \tmy ($self, $subdir) = @_;\n \t$self->wc_path()\n-\t\tor throw Error::Simple(\"bare repository\");\n+\t\tor die \"bare repository\";\n \n \t-d $self->wc_path().'/'.$subdir\n-\t\tor throw Error::Simple(\"subdir not found: $subdir $!\");\n+\t\tor die \"subdir not found: $subdir $!\";\n \t# Of course we will not \"hold\" the subdirectory so anyone\n \t# can delete it now and we will never know. But at least we tried.\n \n@@ -825,13 +825,13 @@ sub hash_and_insert_object {\n \n \tunless (print $out $filename, \"\\n\") {\n \t\t$self->_close_hash_and_insert_object();\n-\t\tthrow Error::Simple(\"out pipe went bad\");\n+\t\tdie \"out pipe went bad\";\n \t}\n \n \tchomp(my $hash = <$in>);\n \tunless (defined($hash)) {\n \t\t$self->_close_hash_and_insert_object();\n-\t\tthrow Error::Simple(\"in pipe went bad\");\n+\t\tdie \"in pipe went bad\";\n \t}\n \n \treturn $hash;\n@@ -873,7 +873,7 @@ sub cat_blob {\n \n \tunless (print $out $sha1, \"\\n\") {\n \t\t$self->_close_cat_blob();\n-\t\tthrow Error::Simple(\"out pipe went bad\");\n+\t\tdie \"out pipe went bad\";\n \t}\n \n \tmy $description = <$in>;\n@@ -900,7 +900,7 @@ sub cat_blob {\n \t\tmy $read = read($in, $blob, $bytesToRead, $bytesRead);\n \t\tunless (defined($read)) {\n \t\t\t$self->_close_cat_blob();\n-\t\t\tthrow Error::Simple(\"in pipe went bad\");\n+\t\t\tdie \"in pipe went bad\";\n \t\t}\n \n \t\t$bytesRead += $read;\n@@ -911,16 +911,16 @@ sub cat_blob {\n \tmy $read = read($in, $newline, 1);\n \tunless (defined($read)) {\n \t\t$self->_close_cat_blob();\n-\t\tthrow Error::Simple(\"in pipe went bad\");\n+\t\tdie \"in pipe went bad\";\n \t}\n \tunless ($read == 1 && $newline eq \"\\n\") {\n \t\t$self->_close_cat_blob();\n-\t\tthrow Error::Simple(\"didn't find newline after blob\");\n+\t\tdie \"didn't find newline after blob\";\n \t}\n \n \tunless (print $fh $blob) {\n \t\t$self->_close_cat_blob();\n-\t\tthrow Error::Simple(\"couldn't write to passed in filehandle\");\n+ die \"couldn't write to passed in filehandle\";\n \t}\n \n \treturn $size;\n@@ -1023,8 +1023,8 @@ sub _temp_cache {\n \tmy $temp_fd = \\$TEMP_FILEMAP{$name};\n \tif (defined $$temp_fd and $$temp_fd->opened) {\n \t\tif ($TEMP_FILES{$$temp_fd}{locked}) {\n-\t\t\tthrow Error::Simple(\"Temp file with moniker '\" .\n-\t\t\t\t$name . \"' already in use\");\n+\t\t\tdie \"Temp file with moniker '\" .\n+\t\t\t\t$name . \"' already in use\";\n \t\t}\n \t} else {\n \t\tif (defined $$temp_fd) {\n@@ -1041,7 +1041,7 @@ sub _temp_cache {\n \n \t\t($$temp_fd, $fname) = File::Temp->tempfile(\n \t\t\t'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,\n-\t\t\t) or throw Error::Simple(\"couldn't open new temp file\");\n+\t\t\t) or die \"couldn't open new temp file\";\n \n \t\t$$temp_fd->autoflush;\n \t\tbinmode $$temp_fd;\n@@ -1052,7 +1052,7 @@ sub _temp_cache {\n \n sub _verify_require {\n \teval { require File::Temp; require File::Spec; };\n-\t$@ and throw Error::Simple($@);\n+\t$@ and die \"$@\";\n }\n \n =item temp_reset ( FILEHANDLE )\n@@ -1065,11 +1065,11 @@ sub temp_reset {\n \tmy ($self, $temp_fd) = _maybe_self(@_);\n \n \ttruncate $temp_fd, 0\n-\t\tor throw Error::Simple(\"couldn't truncate file\");\n+\t\tor die \"couldn't truncate file\";\n \tsysseek($temp_fd, 0, SEEK_SET) and seek($temp_fd, 0, SEEK_SET)\n-\t\tor throw Error::Simple(\"couldn't seek to beginning of file\");\n+\t\tor die \"couldn't seek to beginning of file\";\n \tsysseek($temp_fd, 0, SEEK_CUR) == 0 and tell($temp_fd) == 0\n-\t\tor throw Error::Simple(\"expected file position to be reset\");\n+\t\tor die \"expected file position to be reset\";\n }\n \n =item temp_path ( NAME )\n@@ -1100,8 +1100,8 @@ sub END {\n =head1 ERROR HANDLING\n \n All functions are supposed to throw Perl exceptions in case of errors.\n-See the L module on how to catch those. Most exceptions are mere\n-L instances.\n+These errors are perl scalars which can be caught in the $@ values in\n+eval{} blocks.\n \n However, the C, C and C\n functions suite can throw C exceptions as well: those are\n@@ -1227,7 +1227,7 @@ sub _maybe_self {\n # Check if the command id is something reasonable.\n sub _check_valid_cmd {\n \tmy ($cmd) = @_;\n-\t$cmd =~ /^[a-z0-9A-Z_-]+$/ or throw Error::Simple(\"bad command: $cmd\");\n+\t$cmd =~ /^[a-z0-9A-Z_-]+$/ or die \"bad command: $cmd\";\n }\n \n # Common backend for the pipe creators.\n@@ -1261,7 +1261,7 @@ sub _command_common_pipe {\n \t} else {\n \t\tmy $pid = open($fh, $direction);\n \t\tif (not defined $pid) {\n-\t\t\tthrow Error::Simple(\"open failed: $!\");\n+\t\t\tdie \"open failed: $!\";\n \t\t} elsif ($pid == 0) {\n \t\t\tif (defined $opts{STDERR}) {\n \t\t\t\tclose STDERR;\n-- \n1.7.9.5\n"},{"id":"191727","messageId":"1337411317-14931-3-git-send-email-subs.zero@gmail.com","threadId":"30337","inReplyTo":"1337411317-14931-1-git-send-email-subs.zero@gmail.com","subject":"[PATCH][GIT.PM 3/3] Perl code uses eval{}/die instead of Error::Simple and Git::Error::Command","fromName":"Subho Sankar Banerjee","fromEmail":"subs.zero@gmail.com","sentAt":"2012-05-19T07:08:37Z","receivedAt":"2012-05-19T07:08:37Z","isPatch":true,"sender":{"key":"subs.zero@gmail.com","avatar":"https://gravatar.com/avatar/6159910e05d7650fc7da0a77c233711a014ea449cc5b30f0f065d0017b3357dd?d=mp&s=160"},"body":"\nSigned-off-by: Subho Sankar Banerjee \n---\n git-send-email.perl | 7 +--\n git-svn.perl | 2 +-\n perl/Git.pm | 170 ++++++++++++---------------------------------------\n 3 files changed, 41 insertions(+), 138 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex ef30c55..e56b379 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -26,7 +26,6 @@ use Data::Dumper;\n use Term::ANSIColor;\n use File::Temp qw/ tempdir tempfile /;\n use File::Spec::Functions qw(catfile);\n-use Error qw(:try);\n use Git;\n \n Getopt::Long::Configure qw/ pass_through /;\n@@ -512,7 +511,7 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {\n sub check_file_rev_conflict($) {\n \treturn unless $repo;\n \tmy $f = shift;\n-\ttry {\n+\teval {\n \t\t$repo->command('rev-parse', '--verify', '--quiet', $f);\n \t\tif (defined($format_patch)) {\n \t\t\treturn $format_patch;\n@@ -524,9 +523,7 @@ to produce patches for. Please disambiguate by...\n * Saying \"./$f\" if you mean a file; or\n * Giving --format-patch option if you mean a range.\n EOF\n-\t} catch Git::Error::Command with {\n-\t\treturn 0;\n-\t}\n+\t} or return 0;\n }\n \n # Now that all the defaults are set, process the rest of the command line\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 31d02b5..c299137 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -3714,7 +3714,7 @@ sub find_extra_svn_parents {\n \t\t};\n \t\tif ($@) {\n \t\t\tdie \"An error occurred during merge-base\"\n-\t\t\t\tunless $@->isa(\"Git::Error::Command\");\n+\t\t\t\tunless $@ =~ /command returned error/;\n \n \t\t\twarn \"W: Cannot find common ancestor between \".\n \t\t\t \"@$parents and $merge_tip. Ignoring merge info.\\n\";\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 52777d4..a025f5d 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -97,8 +97,7 @@ increase notwithstanding).\n =cut\n \n \n-use Carp qw(carp croak); # but croak is bad - throw instead\n-use Error qw(:try);\n+use Carp qw(carp croak);\n use Cwd qw(abs_path cwd);\n use IPC::Open2 qw(open2);\n use Fcntl qw(SEEK_SET SEEK_CUR);\n@@ -123,7 +122,6 @@ B - Path to the Git repository.\n \n B - Path to the associated working copy; not strictly required\n as many commands will happily crunch on a bare repository.\n-\n B - Subdirectory in the working copy to work inside.\n Just left undefined if you do not want to limit the scope of operations.\n \n@@ -160,7 +158,7 @@ sub repository {\n \tif (defined $args[0]) {\n \t\tif ($#args % 2 != 1) {\n \t\t\t# Not a hash.\n-\t\t\t$#args == 0 or die \"bad usage\";\n+\t\t\t$#args == 0 or croak \"bad usage\";\n \t\t\t%opts = ( Directory => $args[0] );\n \t\t} else {\n \t\t\t%opts = @args;\n@@ -177,12 +175,10 @@ sub repository {\n \n \t\tmy $search = Git->repository(WorkingCopy => $opts{Directory});\n \t\tmy $dir;\n-\t\ttry {\n+\t\teval {\n \t\t\t$dir = $search->command_oneline(['rev-parse', '--git-dir'],\n-\t\t\t STDERR => 0);\n-\t\t} catch Git::Error::Command with {\n-\t\t\t$dir = undef;\n-\t\t};\n+\t\t\t STDERR => 0);1;\n+\t\t} or $dir = undef and print $@;\n \n \t\tif ($dir) {\n \t\t\t$dir =~ m#^/# or $dir = $opts{Directory} . '/' . $dir;\n@@ -206,15 +202,12 @@ sub repository {\n \n \t\t\tunless (-d \"$dir/refs\" and -d \"$dir/objects\" and -e \"$dir/HEAD\") {\n \t\t\t\t# Mimic git-rev-parse --git-dir error message:\n-\t\t\t\tdie \"fatal: Not a git repository: $dir\";\n+\t\t\t\tcroak \"fatal: Not a git repository: $dir\";\n \t\t\t}\n \t\t\tmy $search = Git->repository(Repository => $dir);\n-\t\t\ttry {\n-\t\t\t\t$search->command('symbolic-ref', 'HEAD');\n-\t\t\t} catch Git::Error::Command with {\n-\t\t\t\t# Mimic git-rev-parse --git-dir error message:\n-\t\t\t\tdie \"fatal: Not a git repository: $dir\";\n-\t\t\t}\n+\t\t\teval {\n+\t\t\t\t$search->command('symbolic-ref', 'HEAD');1;\n+\t\t\t} or croak \"fatal: Not a git repository: $dir\"; # Mimic git-rev-parse --git-dir error message\n \n \t\t\t$opts{Repository} = abs_path($dir);\n \t\t}\n@@ -265,33 +258,20 @@ In both cases, the command's stdin and stderr are the same as the caller's.\n sub command {\n \tmy ($fh, $ctx) = command_output_pipe(@_);\n \n+\tlocal $@;\n \tif (not defined wantarray) {\n-\t\t# Nothing to pepper the possible exception with.\n \t\t_cmd_close($fh, $ctx);\n \n \t} elsif (not wantarray) {\n \t\tlocal $/;\n \t\tmy $text = <$fh>;\n-\t\ttry {\n-\t\t\t_cmd_close($fh, $ctx);\n-\t\t} catch Git::Error::Command with {\n-\t\t\t# Pepper with the output:\n-\t\t\tmy $E = shift;\n-\t\t\t$E->{'-outputref'} = \\$text;\n-\t\t\tthrow $E;\n-\t\t};\n+\teval { _cmd_close($fh, $ctx); 1; } or die $@;\n \t\treturn $text;\n \n \t} else {\n \t\tmy @lines = <$fh>;\n \t\tdefined and chomp for @lines;\n-\t\ttry {\n-\t\t\t_cmd_close($fh, $ctx);\n-\t\t} catch Git::Error::Command with {\n-\t\t\tmy $E = shift;\n-\t\t\t$E->{'-outputref'} = \\@lines;\n-\t\t\tthrow $E;\n-\t\t};\n+\t\teval { _cmd_close($fh, $ctx); 1; } or die $@;\n \t\treturn @lines;\n \t}\n }\n@@ -312,14 +292,7 @@ sub command_oneline {\n \n \tmy $line = <$fh>;\n \tdefined $line and chomp $line;\n-\ttry {\n-\t\t_cmd_close($fh, $ctx);\n-\t} catch Git::Error::Command with {\n-\t\t# Pepper with the output:\n-\t\tmy $E = shift;\n-\t\t$E->{'-outputref'} = \\$line;\n-\t\tthrow $E;\n-\t};\n+\teval { _cmd_close($fh, $ctx); }\tor die $@;\n \treturn $line;\n }\n \n@@ -436,7 +409,7 @@ sub command_close_bidi_pipe {\n \t\t\tif ($!) {\n \t\t\t\tcarp \"error closing pipe: $!\";\n \t\t\t} elsif ($? >> 8) {\n-\t\t\t\tthrow Git::Error::Command($ctx, $? >>8);\n+\t\t\t\tdie $ctx.\" : command returned error : \".($? >> 8).\"\\n\";\n \t\t\t}\n \t\t}\n \t}\n@@ -444,7 +417,7 @@ sub command_close_bidi_pipe {\n \twaitpid $pid, 0;\n \n \tif ($? >> 8) {\n-\t\tthrow Git::Error::Command($ctx, $? >>8);\n+\t\tdie $ctx.\" : command returned error : \".($? >> 8).\"\\n\";\n \t}\n }\n \n@@ -474,7 +447,7 @@ sub command_noisy {\n \t\t_cmd_exec($self, $cmd, @args);\n \t}\n \tif (waitpid($pid, 0) > 0 and $?>>8 != 0) {\n-\t\tthrow Git::Error::Command(join(' ', $cmd, @args), $? >> 8);\n+\t\tdie join(' ', $cmd, @args).\" : command returned error : \".($? >> 8).\"\\n\";\n \t}\n }\n \n@@ -552,10 +525,10 @@ and the directory must exist.\n sub wc_chdir {\n \tmy ($self, $subdir) = @_;\n \t$self->wc_path()\n-\t\tor die \"bare repository\";\n+\t\tor croak \"bare repository\";\n \n \t-d $self->wc_path().'/'.$subdir\n-\t\tor die \"subdir not found: $subdir $!\";\n+\t\tor croak \"subdir not found: $subdir $!\";\n \t# Of course we will not \"hold\" the subdirectory so anyone\n \t# can delete it now and we will never know. But at least we tried.\n \n@@ -629,8 +602,9 @@ sub config_int {\n sub _config_common {\n \tmy ($opts) = shift @_;\n \tmy ($self, $var) = _maybe_self(@_);\n-\n-\ttry {\n+\t\n+\tlocal $@;\n+\teval {\n \t\tmy @cmd = ('config', $opts->{'kind'} ? $opts->{'kind'} : ());\n \t\tunshift @cmd, $self if $self;\n \t\tif (wantarray) {\n@@ -638,15 +612,9 @@ sub _config_common {\n \t\t} else {\n \t\t\treturn command_oneline(@cmd, '--get', $var);\n \t\t}\n-\t} catch Git::Error::Command with {\n-\t\tmy $E = shift;\n-\t\tif ($E->value() == 1) {\n-\t\t\t# Key not found.\n-\t\t\treturn;\n-\t\t} else {\n-\t\t\tthrow $E;\n-\t\t}\n-\t};\n+\t\t1;\n+\t} or $@ =~ /([\\d]+)$/ and (0+$1 == 1) or die $@;\n+\treturn;\n }\n \n =item get_colorbool ( NAME )\n@@ -920,7 +888,7 @@ sub cat_blob {\n \n \tunless (print $fh $blob) {\n \t\t$self->_close_cat_blob();\n- die \"couldn't write to passed in filehandle\";\n+\t\tdie \"couldn't write to passed in filehandle\";\n \t}\n \n \treturn $size;\n@@ -1023,7 +991,7 @@ sub _temp_cache {\n \tmy $temp_fd = \\$TEMP_FILEMAP{$name};\n \tif (defined $$temp_fd and $$temp_fd->opened) {\n \t\tif ($TEMP_FILES{$$temp_fd}{locked}) {\n-\t\t\tdie \"Temp file with moniker '\" .\n+\t\t\tcroak \"Temp file with moniker '\" .\n \t\t\t\t$name . \"' already in use\";\n \t\t}\n \t} else {\n@@ -1103,72 +1071,13 @@ All functions are supposed to throw Perl exceptions in case of errors.\n These errors are perl scalars which can be caught in the $@ values in\n eval{} blocks.\n \n-However, the C, C and C\n-functions suite can throw C exceptions as well: those are\n-thrown when the external command returns an error code and contain the error\n-code as well as access to the captured command's output. The exception class\n-provides the usual C and C (command's exit code) methods and\n-in addition also a C method that returns either an array or a\n-string with the captured command output (depending on the original function\n-call context; C returns C) and $ which\n-returns the command and its arguments (but without proper quoting).\n-\n-Note that the C functions cannot throw this exception since\n-it has no idea whether the command failed or not. You will only find out\n-at the time you C the pipe; if you want to have that automated,\n-use C, which can throw the exception.\n-\n =cut\n \n-{\n-\tpackage Git::Error::Command;\n-\n-\t@Git::Error::Command::ISA = qw(Error);\n-\n-\tsub new {\n-\t\tmy $self = shift;\n-\t\tmy $cmdline = '' . shift;\n-\t\tmy $value = 0 + shift;\n-\t\tmy $outputref = shift;\n-\t\tmy(@args) = ();\n-\n-\t\tlocal $Error::Depth = $Error::Depth + 1;\n-\n-\t\tpush(@args, '-cmdline', $cmdline);\n-\t\tpush(@args, '-value', $value);\n-\t\tpush(@args, '-outputref', $outputref);\n-\n-\t\t$self->SUPER::new(-text => 'command returned error', @args);\n-\t}\n-\n-\tsub stringify {\n-\t\tmy $self = shift;\n-\t\tmy $text = $self->SUPER::stringify;\n-\t\t$self->cmdline() . ': ' . $text . ': ' . $self->value() . \"\\n\";\n-\t}\n-\n-\tsub cmdline {\n-\t\tmy $self = shift;\n-\t\t$self->{'-cmdline'};\n-\t}\n-\n-\tsub cmd_output {\n-\t\tmy $self = shift;\n-\t\tmy $ref = $self->{'-outputref'};\n-\t\tdefined $ref or undef;\n-\t\tif (ref $ref eq 'ARRAY') {\n-\t\t\treturn @$ref;\n-\t\t} else { # SCALAR\n-\t\t\treturn $$ref;\n-\t\t}\n-\t}\n-}\n-\n =over 4\n \n =item git_cmd_try { CODE } ERRMSG\n \n-This magical statement will automatically catch any C\n+This magical statement will automatically catch any \n exceptions thrown by C and make your program die with C\n on its lips; the message will have %s substituted for the command line\n and %d for the exit status. This statement is useful mostly for producing\n@@ -1184,22 +1093,20 @@ sub git_cmd_try(&$) {\n \tmy ($code, $errmsg) = @_;\n \tmy @result;\n \tmy $err;\n+\tmy $err_string;\n+\tmy $err_value;\n \tmy $array = wantarray;\n-\ttry {\n+\teval {\n \t\tif ($array) {\n \t\t\t@result = &$code;\n \t\t} else {\n \t\t\t$result[0] = &$code;\n-\t\t}\n-\t} catch Git::Error::Command with {\n-\t\tmy $E = shift;\n-\t\t$err = $errmsg;\n-\t\t$err =~ s/\\%s/$E->cmdline()/ge;\n-\t\t$err =~ s/\\%d/$E->value()/ge;\n-\t\t# We can't croak here since Error.pm would mangle\n-\t\t# that to Error::Simple.\n-\t};\n-\t$err and croak $err;\n+\t\t}1;\n+\t} or $@ =~ /^([a-zA-Z ]+) : .* : ([\\d]+)$/\n+\tand ($err, $err_string, $err_value) = ($errmsg, $1, $2)\n+\tand $err =~ s/\\%s/$1/ge and $err =~ s/\\%d/$2/ge; \n+ \n+ \t$err and croak $err;\n \treturn $array ? @result : $result[0];\n }\n \n@@ -1227,7 +1134,7 @@ sub _maybe_self {\n # Check if the command id is something reasonable.\n sub _check_valid_cmd {\n \tmy ($cmd) = @_;\n-\t$cmd =~ /^[a-z0-9A-Z_-]+$/ or die \"bad command: $cmd\";\n+\t$cmd =~ /^[a-z0-9A-Z_-]+$/ or croak \"bad command: $cmd\";\n }\n \n # Common backend for the pipe creators.\n@@ -1309,8 +1216,7 @@ sub _cmd_close {\n \t\t\t# It's just close, no point in fatalities\n \t\t\tcarp \"error closing pipe: $!\";\n \t\t} elsif ($? >> 8) {\n-\t\t\t# The caller should pepper this.\n-\t\t\tthrow Git::Error::Command($ctx, $? >> 8);\n+\t\t\tdie $ctx.\" : command returned error : \".($? >> 8).\"\\n\";\n \t\t}\n \t\t# else we might e.g. closed a live stream; the command\n \t\t# dying of SIGPIPE would drive us here.\n-- \n1.7.9.5\n"},{"id":"191732","messageId":"4FB76A21.7000801@pileofstuff.org","threadId":"30337","inReplyTo":"1337411317-14931-2-git-send-email-subs.zero@gmail.com","subject":"Re: [PATCH][GIT.PM 2/3] Getting rid of throwing Error::Simple objects in favour of simple Perl scalars which can be caught in eval{} blocks","fromName":"Andrew Sayers","fromEmail":"andrew-git@pileofstuff.org","sentAt":"2012-05-19T09:38:41Z","receivedAt":"2012-05-19T09:38:41Z","isPatch":true,"sender":{"key":"andrew-git@pileofstuff.org","avatar":null},"body":"I'll limit myself to a style review here - other people can say better\nthan me about the deeper issues.\n\nOn 19/05/12 08:08, Subho Sankar Banerjee wrote:\n\n> @@ -160,7 +160,7 @@ sub repository {\n> \tif (defined $args[0]) {\n> \t\tif ($#args % 2 != 1) {\n> \t\t\t# Not a hash.\n> -\t\t\t$#args == 0 or throw Error::Simple(\"bad usage\");\n> +\t\t\t$#args == 0 or die \"bad usage\";\n\nThis is valid and no worse than before, but I find this use of the \"or\"\noperator slightly confusing. I find it easier to read either:\n\n\t or \n\tOR:\n\t unless \n\nFor example:\n\n\tdo_something($foo) or die \"couldn't do_something with '$foo'\";\n\tOR:\n\tdie \"'$foo' is not a something\" unless is_something($foo);\n\n\n> @@ -1041,7 +1041,7 @@ sub _temp_cache {\n> \n> \t\t($$temp_fd, $fname) = File::Temp->tempfile(\n> \t\t\t'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,\n> -\t\t\t) or throw Error::Simple(\"couldn't open new temp file\");\n> +\t\t\t) or die \"couldn't open new temp file\";\n\nThis is a good example of where I think \"or\" is appropriate.\n\nThink of it in terms of an English sentence. Which of these would you\nfind easier to read:\n\n\tIt is raining or go out and play\n\tOR:\n\tGo out and play unless it is raining\n\n\tFind your umbrella or cancel the trip\n\tOR:\n\tCancel the trip unless find your umbrella\n\n\nA bit of background for people who aren't (primarily) Perl programmers:\n\nAs an expressive language that promotes \"more than one way to do it\",\nPerl has a long tradition of supporting many redundant ways of spelling\n\"if (...) { ... }\". Common examples include:\n\n\tif ( $x ) { do_something() }\n\tdo_something() if $x;\n\n\tunless ( $x ) { do_something() }\n\tdo_something() unless $x;\n\n\t$x && do_something();\n\t$x || do_something();\n\n\t$x and do_something();\n\t$x or do_something();\n\nSometimes people find very practical reasons why these aren't good\nprogramming practice, but the rest of the time everyone just argues\nabout whether they're good grammar.\n\nThe \"&&\" and \"||\" operators are an example of bad programming practice -\nthese operators have relatively high precedence, so tend to behave\nunintuitively when used in (often regrettably) complex ways. The \"and\"\nand \"or\" operators behave just like \"&&\" and \"||\", but with a precedence\nlow enough to avoid weirdness. See [1] for an example.\n\nSome people consider anything but a traditional prefix-if() statement to\nbe bad grammar (I believe \"Perl Best Practices\" makes the argument,\nwhich is definitive for many people). Other people say anything in the\nlanguage is by definition fair game. The rest of us spend a lot of time\nmaking arguments like the above, and frankly I think we gain more from\nthe debate than the conclusion.\n\n\t- Andrew\n\n[1] http://perldoc.perl.org/perlop.html#C-style-Logical-Defined-Or\n"},{"id":"191973","messageId":"CAB3zAY3nVDiBH6kJKK9YTXKsaFZZnUz7AAFh5z+J0VhXHjYiMQ@mail.gmail.com","threadId":"30337","inReplyTo":"4FB76A21.7000801@pileofstuff.org","subject":"Re: [PATCH][GIT.PM 2/3] Getting rid of throwing Error::Simple objects in favour of simple Perl scalars which can be caught in eval{} blocks","fromName":"Subho Banerjee","fromEmail":"subs.zero@gmail.com","sentAt":"2012-05-23T11:02:00Z","receivedAt":"2012-05-23T11:02:00Z","isPatch":true,"sender":{"key":"subs.zero@gmail.com","avatar":"https://gravatar.com/avatar/6159910e05d7650fc7da0a77c233711a014ea449cc5b30f0f065d0017b3357dd?d=mp&s=160"},"body":"Hi,\nThe semantic\n>         unless \nworks well when the part of the code is a singular statement.\nBut it is of ungainly when there are a couple of statements to be\nexecuted as a block. In this case, I believe that a the conjunctive\n,,or''/,,and'' statement makes more sense. In the sense -\n or and \nI believe this is easier to read compared to -\n and unless \nespecially if you have a larger block of commands to execute in case\nof the failure. I believe the easiest to read would be a classical C\nstyled if() block, but that would make the code more \"verbose\" :-)\n\nBut I am open to the change of the ,,or''s to ,,unless\"s. They are\njust cosmetic changes. I can submit patches to that effect if that's\nwhat you guys want.\n\nCheers,\nSubho.\n\nOn Sat, May 19, 2012 at 3:08 PM, Andrew Sayers\n wrote:\n> I'll limit myself to a style review here - other people can say better\n> than me about the deeper issues.\n>\n> On 19/05/12 08:08, Subho Sankar Banerjee wrote:\n> \n>> @@ -160,7 +160,7 @@ sub repository {\n>>       if (defined $args[0]) {\n>>               if ($#args % 2 != 1) {\n>>                       # Not a hash.\n>> -                     $#args == 0 or throw Error::Simple(\"bad usage\");\n>> +                     $#args == 0 or die \"bad usage\";\n>\n> This is valid and no worse than before, but I find this use of the \"or\"\n> operator slightly confusing.  I find it easier to read either:\n>\n>         or \n>        OR:\n>         unless \n>\n> For example:\n>\n>        do_something($foo) or die \"couldn't do_something with '$foo'\";\n>        OR:\n>        die \"'$foo' is not a something\" unless is_something($foo);\n>\n> \n>> @@ -1041,7 +1041,7 @@ sub _temp_cache {\n>>\n>>               ($$temp_fd, $fname) = File::Temp->tempfile(\n>>                       'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,\n>> -                     ) or throw Error::Simple(\"couldn't open new temp file\");\n>> +                     ) or die \"couldn't open new temp file\";\n>\n> This is a good example of where I think \"or\" is appropriate.\n>\n> Think of it in terms of an English sentence.  Which of these would you\n> find easier to read:\n>\n>        It is raining or go out and play\n>        OR:\n>        Go out and play unless it is raining\n>\n>        Find your umbrella or cancel the trip\n>        OR:\n>        Cancel the trip unless find your umbrella\n>\n>\n> A bit of background for people who aren't (primarily) Perl programmers:\n>\n> As an expressive language that promotes \"more than one way to do it\",\n> Perl has a long tradition of supporting many redundant ways of spelling\n> \"if (...) { ... }\".  Common examples include:\n>\n>        if ( $x ) { do_something() }\n>        do_something() if $x;\n>\n>        unless ( $x ) { do_something() }\n>        do_something() unless $x;\n>\n>        $x && do_something();\n>        $x || do_something();\n>\n>        $x and do_something();\n>        $x or do_something();\n>\n> Sometimes people find very practical reasons why these aren't good\n> programming practice, but the rest of the time everyone just argues\n> about whether they're good grammar.\n>\n> The \"&&\" and \"||\" operators are an example of bad programming practice -\n> these operators have relatively high precedence, so tend to behave\n> unintuitively when used in (often regrettably) complex ways.  The \"and\"\n> and \"or\" operators behave just like \"&&\" and \"||\", but with a precedence\n> low enough to avoid weirdness.  See [1] for an example.\n>\n> Some people consider anything but a traditional prefix-if() statement to\n> be bad grammar (I believe \"Perl Best Practices\" makes the argument,\n> which is definitive for many people).  Other people say anything in the\n> language is by definition fair game.  The rest of us spend a lot of time\n> making arguments like the above, and frankly I think we gain more from\n> the debate than the conclusion.\n>\n>        - Andrew\n>\n> [1] http://perldoc.perl.org/perlop.html#C-style-Logical-Defined-Or\n"},{"id":"192013","messageId":"4FBD3C2D.3010107@pileofstuff.org","threadId":"30337","inReplyTo":"CAB3zAY3nVDiBH6kJKK9YTXKsaFZZnUz7AAFh5z+J0VhXHjYiMQ@mail.gmail.com","subject":"Re: [PATCH][GIT.PM 2/3] Getting rid of throwing Error::Simple objects in favour of simple Perl scalars which can be caught in eval{} blocks","fromName":"Andrew Sayers","fromEmail":"andrew-git@pileofstuff.org","sentAt":"2012-05-23T19:36:13Z","receivedAt":"2012-05-23T19:36:13Z","isPatch":true,"sender":{"key":"andrew-git@pileofstuff.org","avatar":null},"body":"On 23/05/12 12:02, Subho Banerjee wrote:\n> Hi,\n> The semantic\n>> unless \n> works well when the part of the code is a singular statement.\n> But it is of ungainly when there are a couple of statements to be\n> executed as a block. In this case, I believe that a the conjunctive\n> ,,or''/,,and'' statement makes more sense. In the sense -\n> or and \n> I believe this is easier to read compared to -\n> and unless \n> especially if you have a larger block of commands to execute in case\n> of the failure. I believe the easiest to read would be a classical C\n> styled if() block, but that would make the code more \"verbose\" :-)\n\nTo be honest, I drop straight back to C-style if() statements as soon as\nI have to start thinking consciously about precedence rules (where by\n\"thinking\" I mean \"creating bugs then failing to see them under my\nnose\"). I also don't think anyone would object if you wanted to stick\nwith classic if() statements everywhere - colloquialisms are supported,\nnot required :)\n\nSo long as you're aware this is an exception to the rule about matching\nthe style of surrounding code, I'm personally quite relaxed about fixing\nthese specific instances. If nobody else has any opinions, maybe hold\noff and see how much change you're planning to make elsewhere? No sense\ngetting too attached to code you might have to throw away.\n\n\t- Andrew\n"}]}