Re: [PATCH v3 2/2] column: guard against negative padding
- From
Rubén Justo <rjusto@gmail.com>
- Date
- Feb 13, 2024, 23:25 UTC
- Message-ID
- <8acde766-e2cd-4901-b665-f677cd15295d@gmail.com>
- In-Reply-To
- <xmqqle7o5f34.fsf@gitster.g>
On 13-feb-2024 11:39:11, Junio C Hamano wrote:
Show 18 quoted lines
> Rubén Justo <rjusto@gmail.com> writes: > > >> This one happens to be safe currently because "git tag" passes 2 in > >> opts->padding, but I do not think this is needed. > > > > At first glance, I also thought this was not necessary. > > > > However, callers of run_column_filter() might forget to check the return > > value, and the BUG() triggered by the underlying process could be buried > > and ignored. Having the BUG() here, in the same process, makes it more > > noticeable. > > The point of BUG() is to help developers catch the silly breakage > before it excapes from the lab, and we can expect these careless > developers to ignore the return value. But "column --padding=-1" > invoked as a subprocess will show a human-readable error message > to such a developer, so it is less important than the BUG() in the > other place.
Thinking again about this; you are right. That BUG() in run_column_filter() does not make sense in this series.
It is addressing a different error, and perhaps a solution could be:
--- >8 --- Subject: [PATCH] tag: error when git-column fails
If the user asks for the list of tags to be displayed in columns ("--columns"), a child git-column process is used to format the output as expected.
In a rare situation where we encounter a problem spawning that child process, we will work erroneously.
Make noticeable we're having a problem executing git-column, so the user can act accordingly.
Signed-off-by: Rubén Justo <rjusto@gmail.com> --- builtin/tag.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/tag.c b/builtin/tag.c index 37473ac21f..30532b76d5 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -530,7 +530,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix) struct column_options copts; memset(&copts, 0, sizeof(copts)); copts.padding = 2; - run_column_filter(colopts, &copts); + if (run_column_filter(colopts, &copts)) + die(_("could not start 'git column'") } filter.name_patterns = argv; ret = list_tags(&filter, sorting, &format);
-- 2.43.0