Re: [PATCH 6/7] [RFC] entry: don't show "Filtering content: ... done." line in case of errors
On Tue, Jun 22, 2021 at 09:52:32PM -0400, Taylor Blau wrote:
Show 23 quoted lines
> On Mon, Jun 21, 2021 at 08:32:56PM +0200, René Scharfe wrote:
> > Am 20.06.21 um 22:03 schrieb SZEDER Gábor:
> > > RFC!! Alas, not calling stop_progress() on error has drawbacks:
> > >
> > > - All memory allocated for the progress bar is leaked.
> > > - This progress line remains "active", in the sense that if we were
> > > to start a new progress later in the same git process, then with
> > > GIT_TEST_CHECK_PROGRESS it would trigger the other BUG() catching
> > > nested/overlapping progresses.
> > >
> > > Do we care?! TBH I don't :)
> > > Anyway, if we do, then we might need some sort of an abort_progress()
> > > function...
> >
> > I think the abort_progress() idea makes sense; to clean up allocations,
> > tell the user what happened and avoid the BUG(). Showing just
> > "aborted" instead of "done" should suffice here -- the explanation is
> > given a few lines later ("'foo' was not filtered properly").
>
> Very well put. I concur that having an abort_progress() API makes sense
> for all of the reasons that you suggest, but also because we shouldn't
> encourage not using what seems like an appropriate API in order to not
> fail tests when GIT_TEST_CHECK_PROGRESS is set.Ah, damn, I was hoping that I can get away with it :)