git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] column: fix parsing of the '--nl' option

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Aug 18, 2021, 11:48 UTC
Message-ID
<nycvar.QRO.7.76.6.2108181347010.55@tvgsbejvaqbjf.bet>
In-Reply-To
<20210818092456.3045808-1-szeder.dev@gmail.com>
Hi Gábor,
On Wed, 18 Aug 2021, SZEDER Gábor wrote:
Show 17 quoted lines
> 'git column's '--nl' option can be used to specify a "string to be
> printed at the end of each line" (quoting the man page), but this
> option and its mandatory argument has been parsed as OPT_INTEGER since
> the introduction of the command in 7e29b8254f (Add column layout
> skeleton and git-column, 2012-04-21).  Consequently, any non-number
> argument is rejected by parse-options, and any number other than 0
> leads to segfault:
>
>   $ printf "%s\n" one two |git column --mode=plain --nl=foo
>   error: option `nl' expects a numerical value
>   $ printf "%s\n" one two |git column --mode=plain --nl=42
>   Segmentation fault (core dumped)
>   $ printf "%s\n" one two |git column --mode=plain --nl=0
>   one
>   two
>
> Parse this option as OPT_STRING.
Whoa. Nice catch. I have just one suggestion about the patch, below.
Show 55 quoted lines
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>  Documentation/git-column.txt |  2 +-
>  builtin/column.c             |  2 +-
>  t/t9002-column.sh            | 18 ++++++++++++++++++
>  3 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-column.txt b/Documentation/git-column.txt
> index f58e9c43e6..6cea9ab463 100644
> --- a/Documentation/git-column.txt
> +++ b/Documentation/git-column.txt
> @@ -39,7 +39,7 @@ OPTIONS
>  --indent=<string>::
>  	String to be printed at the beginning of each line.
>
> ---nl=<N>::
> +--nl=<string>::
>  	String to be printed at the end of each line,
>  	including newline character.
>
> diff --git a/builtin/column.c b/builtin/column.c
> index 40d4b3bee2..158fdf53d9 100644
> --- a/builtin/column.c
> +++ b/builtin/column.c
> @@ -29,7 +29,7 @@ int cmd_column(int argc, const char **argv, const char *prefix)
>  		OPT_INTEGER(0, "raw-mode", &colopts, N_("layout to use")),
>  		OPT_INTEGER(0, "width", &copts.width, N_("maximum width")),
>  		OPT_STRING(0, "indent", &copts.indent, N_("string"), N_("padding space on left border")),
> -		OPT_INTEGER(0, "nl", &copts.nl, N_("padding space on right border")),
> +		OPT_STRING(0, "nl", &copts.nl, N_("string"), N_("padding space on right border")),
>  		OPT_INTEGER(0, "padding", &copts.padding, N_("padding space between columns")),
>  		OPT_END()
>  	};
> diff --git a/t/t9002-column.sh b/t/t9002-column.sh
> index 89983527b6..6d3dbde3fe 100755
> --- a/t/t9002-column.sh
> +++ b/t/t9002-column.sh
> @@ -42,6 +42,24 @@ EOF
>  	test_cmp expected actual
>  '
>
> +test_expect_success '--nl' '
> +	cat >expected <<\EOF &&
> +oneZ
> +twoZ
> +threeZ
> +fourZ
> +fiveZ
> +sixZ
> +sevenZ
> +eightZ
> +nineZ
> +tenZ
> +elevenZ
> +EOF
Or maybe we can do this instead?
	sed "s/\$/Z" <lista >expected &&

Much shorter, and less error-prone (in case any `--run=[...]` option would change what is in `lista` in the future).

Ciao, Dscho

Show 11 quoted lines
> +	git column --nl="Z$LF" --mode=plain <lista >actual &&
> +	test_cmp expected actual
> +'
> +
>  test_expect_success '80 columns' '
>  	cat >expected <<\EOF &&
>  one    two    three  four   five   six    seven  eight  nine   ten    eleven
> --
> 2.33.0.453.gc5e41af357
>
>
Previous: SZEDER GáborNext: Junio C Hamano
Message 2 of 5 in “column: fix parsing of the '--nl' option”
  1. column: fix parsing of the '--nl' optionSZEDER Gábor, Aug 18, 2021
  2. Johannes SchindelinAug 18, 2021
  3. Junio C HamanoAug 18, 2021
  4. SZEDER GáborAug 26, 2021
  5. Junio C HamanoAug 26, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.