Hi Junio, On Tue, 7 Sep 2021, Junio C Hamano wrote: > "Miriam R." <mirucam@xxxxxxxxx> writes: > > >> However, I still don't like that we play such a `dup2()` game. I gave it a > >> quick try to avoid it (see the diff below, which corresponds to the commit > >> I pushed up as `git-bisect-work-part4-v7` to > >> https://github.com/dscho/git), which still could benefit from a bit of > >> polishing (maybe we should rethink the object model and extend/rename > >> `bisect_terms` to `bisect_state` and accumulate more fields, such as > >> `out_fd`. > >> > >> Obviously this will need to be cleaned up, and while I would _love_ to see > >> this make it into your next iteration, ultimately it is up to you, Miriam, > >> to decide whether you want to build on my diff (quite possibly making the > >> entire object model of the bisect part of Git's code more elegant and more > >> maintainable), and up to you, Junio, to decide whether you would be > >> willing to accept the patch series without this refactoring. > > If the code paths involved are shallow and narrow enough that not > too many existing callers need to start passing FILE *stdout down > (from the looks of your illustration patch, it does not seem to be > too bad), I do not mind a series that is a bit longer than the > current 6-patch series that has a preliminary enhancement step that > allows callers to pass their own "FILE *" for output destination > before the main part of the topic. My impression, from the diff that I sent, is that this is too deep and wide, and indeed needs a follow-up patch series as indicated by Miriam. My preference would be (as I hinted at) to accumulate relevant data (such as the terms and, yes, the `FILE *`) into a `struct bisect_state` and pass that around. Sort of a light-weight object-oriented design, similar to how we do things in `builtin/am.c` with `struct am_state`. Thanks, Dscho
