2018-03-13 22:26 GMT+03:00 Martin Ågren <martin.agren@xxxxxxxxx>: > Hi Olga > > On 13 March 2018 at 11:25, Оля Тележная <olyatelezhnaya@xxxxxxxxx> wrote: >> The main idea of the patch is, if you want to format the output by >> ref-filter, you should have an ability to work with errors by yourself >> if you want to. >> So I decided not to touch signature of show_ref_array_item(), but to >> move all printing (I mean errors) to it. So that we could invoke >> format_ref_array_item() and be sure that we cold handle errors by >> ourselves. >> >> The patch is not finished, but I decided to show it to you. There are >> still many places where we could die in the middle of formatting >> process. But, if you like the general idea, I will finish and re-send >> it. >> >> Another question is about verify_ref_format(). Do we need to allow its >> users also to manage errors by themselves? I left the old scenario, >> printing everything in verify_ref_format() and die. If you have better >> ideas, please share them. > > I think it is a good idea to stop die-ing in "libgit". This seems like a > good way of achieving that, or isolating the issue. Do you have any > particular use-case for this, i.e., are you setting up the stage for a > patch "5" where you add a new user of one of these? Yes, I want to reuse formatting part in cat-file command. In cat-file sometimes we have an error but we want to continue our work and check all other sha1s. But, anyway, I find these changes useful not only for cat-file. > > I do wonder whether a helper function to call strbuf_addstr() and return > -1 would be a good idea though. I mentioned it in patch 2, then with > patches 3 and 4, it started to seem like a reasonably good idea. It > would be a shame if this sort of "boilerplate" for handling errors could > have an impact on code clarity / obviousness. I am also not sure if the code will be intuitive enough. > > Another issue is whether passing NULL for an error-strbuf should be a > way of saying "I don't care; die() so I do not have to". Well, right now > I guess passing NULL would indeed terminate the program. ;-) Such a > construct might be another reason for providing error_strbuf_addstr()... > Of course, it also means we keep die-ing in libgit.. > > I feel I'm just talking out loud. Maybe you find my input useful. I do so! Thanks a lot. I fixed all that you mentioned, you could find new code here if you want: https://github.com/telezhnaya/git/commits/prepare I will not re-send it to the mailing list now. I want to finish the patch at first, there are still some die() calls. If anyone has other thoughts or ideas - please share them. > > Martin
