{"thread":{"id":"48031","subject":"[RFC 0/4] ref-filter: remove printing from formatting logic","startedAt":"2018-03-13T10:25:54Z","lastAt":"2018-03-14T13:44:09Z","messageCount":3,"participants":["Оля Тележная","Martin Ågren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"341534","messageId":"CAL21BmkmXKzdwYHu1pNxuHhaxqei4ekVbutbuv2jmv6=GgcG_A@mail.gmail.com","threadId":"48031","inReplyTo":null,"subject":"[RFC 0/4] ref-filter: remove printing from formatting logic","fromName":"Оля Тележная","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-03-13T10:25:47Z","receivedAt":"2018-03-13T10:25:54Z","isPatch":false,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"The main idea of the patch is, if you want to format the output by\nref-filter, you should have an ability to work with errors by yourself\nif you want to.\nSo I decided not to touch signature of show_ref_array_item(), but to\nmove all printing (I mean errors) to it. So that we could invoke\nformat_ref_array_item() and be sure that we cold handle errors by\nourselves.\n\nThe patch is not finished, but I decided to show it to you. There are\nstill many places where we could die in the middle of formatting\nprocess. But, if you like the general idea, I will finish and re-send\nit.\n\nAnother question is about verify_ref_format(). Do we need to allow its\nusers also to manage errors by themselves? I left the old scenario,\nprinting everything in verify_ref_format() and die. If you have better\nideas, please share them.\n\nThanks a lot!\nOlga\n"},{"id":"341573","messageId":"CAN0heSp-Vsvr95zc3J0iokRMJt74pxFH7OsoiR_3gnkMxpAAKw@mail.gmail.com","threadId":"48031","inReplyTo":"CAL21BmkmXKzdwYHu1pNxuHhaxqei4ekVbutbuv2jmv6=GgcG_A@mail.gmail.com","subject":"Re: [RFC 0/4] ref-filter: remove printing from formatting logic","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-03-13T19:26:23Z","receivedAt":"2018-03-13T19:26:28Z","isPatch":false,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Hi Olga\n\nOn 13 March 2018 at 11:25, Оля Тележная wrote:\n> The main idea of the patch is, if you want to format the output by\n> ref-filter, you should have an ability to work with errors by yourself\n> if you want to.\n> So I decided not to touch signature of show_ref_array_item(), but to\n> move all printing (I mean errors) to it. So that we could invoke\n> format_ref_array_item() and be sure that we cold handle errors by\n> ourselves.\n>\n> The patch is not finished, but I decided to show it to you. There are\n> still many places where we could die in the middle of formatting\n> process. But, if you like the general idea, I will finish and re-send\n> it.\n>\n> Another question is about verify_ref_format(). Do we need to allow its\n> users also to manage errors by themselves? I left the old scenario,\n> printing everything in verify_ref_format() and die. If you have better\n> ideas, please share them.\n\nI think it is a good idea to stop die-ing in \"libgit\". This seems like a\ngood way of achieving that, or isolating the issue. Do you have any\nparticular use-case for this, i.e., are you setting up the stage for a\npatch \"5\" where you add a new user of one of these?\n\nI do wonder whether a helper function to call strbuf_addstr() and return\n-1 would be a good idea though. I mentioned it in patch 2, then with\npatches 3 and 4, it started to seem like a reasonably good idea. It\nwould be a shame if this sort of \"boilerplate\" for handling errors could\nhave an impact on code clarity / obviousness.\n\nAnother issue is whether passing NULL for an error-strbuf should be a\nway of saying \"I don't care; die() so I do not have to\". Well, right now\nI guess passing NULL would indeed terminate the program. ;-) Such a\nconstruct might be another reason for providing error_strbuf_addstr()...\nOf course, it also means we keep die-ing in libgit..\n\nI feel I'm just talking out loud. Maybe you find my input useful.\n\nMartin\n"},{"id":"341632","messageId":"CAL21BmkgeytOKJ64NRJwhvo4Bk19iOVxxi-sOu-LWdf5rFfurg@mail.gmail.com","threadId":"48031","inReplyTo":"CAN0heSp-Vsvr95zc3J0iokRMJt74pxFH7OsoiR_3gnkMxpAAKw@mail.gmail.com","subject":"Re: [RFC 0/4] ref-filter: remove printing from formatting logic","fromName":"Оля Тележная","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-03-14T13:44:02Z","receivedAt":"2018-03-14T13:44:09Z","isPatch":false,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"2018-03-13 22:26 GMT+03:00 Martin Ågren :\n> Hi Olga\n>\n> On 13 March 2018 at 11:25, Оля Тележная wrote:\n>> The main idea of the patch is, if you want to format the output by\n>> ref-filter, you should have an ability to work with errors by yourself\n>> if you want to.\n>> So I decided not to touch signature of show_ref_array_item(), but to\n>> move all printing (I mean errors) to it. So that we could invoke\n>> format_ref_array_item() and be sure that we cold handle errors by\n>> ourselves.\n>>\n>> The patch is not finished, but I decided to show it to you. There are\n>> still many places where we could die in the middle of formatting\n>> process. But, if you like the general idea, I will finish and re-send\n>> it.\n>>\n>> Another question is about verify_ref_format(). Do we need to allow its\n>> users also to manage errors by themselves? I left the old scenario,\n>> printing everything in verify_ref_format() and die. If you have better\n>> ideas, please share them.\n>\n> I think it is a good idea to stop die-ing in \"libgit\". This seems like a\n> good way of achieving that, or isolating the issue. Do you have any\n> particular use-case for this, i.e., are you setting up the stage for a\n> patch \"5\" where you add a new user of one of these?\n\nYes, I want to reuse formatting part in cat-file command. In cat-file\nsometimes we have an error but we want to continue our work and check\nall other sha1s. But, anyway, I find these changes useful not only for\ncat-file.\n\n>\n> I do wonder whether a helper function to call strbuf_addstr() and return\n> -1 would be a good idea though. I mentioned it in patch 2, then with\n> patches 3 and 4, it started to seem like a reasonably good idea. It\n> would be a shame if this sort of \"boilerplate\" for handling errors could\n> have an impact on code clarity / obviousness.\n\nI am also not sure if the code will be intuitive enough.\n\n>\n> Another issue is whether passing NULL for an error-strbuf should be a\n> way of saying \"I don't care; die() so I do not have to\". Well, right now\n> I guess passing NULL would indeed terminate the program. ;-) Such a\n> construct might be another reason for providing error_strbuf_addstr()...\n> Of course, it also means we keep die-ing in libgit..\n>\n> I feel I'm just talking out loud. Maybe you find my input useful.\n\nI do so! Thanks a lot.\nI fixed all that you mentioned, you could find new code here if you want:\nhttps://github.com/telezhnaya/git/commits/prepare\nI will not re-send it to the mailing list now. I want to finish the\npatch at first, there are still some die() calls.\nIf anyone has other thoughts or ideas - please share them.\n\n>\n> Martin\n"}]}