Re: [PATCH 1/4] column: Optionally keep empty lines in cols/rows mode

[Karel Zak <kzak@redhat.com>]

On Sun, Sep 20, 2020 at 02:55:18PM +0200, Lennard Hofmann wrote:
> The following patches remove the unnecessary restriction of the -L option.

Good idea.

> patch below simply moves the existing logic to a new function `add_entry()` that
> gets called with an empty wcs if the -L option is present.
> 
> I am very new to C and mailing lists so I appreciate any feedback.

No problem :-)

> 
> text-utils/column.c
> | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/text-utils/column.c b/text-utils/column.c
> index 238dbab41..bc7851472 100644
> --- a/text-utils/column.c
> +++ b/text-utils/column.c
> @@ -487,6 +487,21 @@ static int add_emptyline_to_table(struct column_control *ctl)
>  	return 0;
>  }
> 
> +static void add_entry(struct column_control *ctl, size_t *maxents, wchar_t *wcs)
> +{
> +	if (ctl->nents <= *maxents) {
> +		*maxents += 1000;
> +		ctl->ents = xrealloc(ctl->ents, *maxents * sizeof(wchar_t *));
> +	}
> +	ctl->ents[ctl->nents] = wcs;

It would be more robust to add also

    ctl->nents++;

to this function.

> +}
> +
> +static void add_empty_entry(struct column_control *ctl, size_t *maxents)
> +{
> +	add_entry(ctl, maxents, mbs_to_wcs(""));
> +	ctl->nents++;
> +}
> +
>  static int read_input(struct column_control *ctl, FILE *fp)
>  {
>  	char *buf = NULL;
> @@ -512,8 +527,12 @@ static int read_input(struct column_control *ctl, FILE *fp)
>  				*p = '\0';
>  		}
>  		if (!str || !*str) {
> -			if (ctl->mode == COLUMN_MODE_TABLE && ctl->tab_empty_lines)
> -				add_emptyline_to_table(ctl);
> +			if (ctl->tab_empty_lines) {
> +				if (ctl->mode == COLUMN_MODE_TABLE)
> +					add_emptyline_to_table(ctl);
> +				else
> +					add_empty_entry(ctl, &maxents);

It seems add_empty_entry() is unnecessary. All you need is:

 else {
    if (!entry)
        empty = mbs_to_wcs("");
    add_entry(ctl, maxents, empty);
 }

and you will also resolve the issue with duplicate mbs_to_wcs("") 
(your patch 2/4).

> +			}
>  			continue;
>  		}
> 
> @@ -539,12 +558,7 @@ static int read_input(struct column_control *ctl, FILE *fp)
> 
>  		case COLUMN_MODE_FILLCOLS:
>  		case COLUMN_MODE_FILLROWS:
> -			if (ctl->nents <= maxents) {
> -				maxents += 1000;
> -				ctl->ents = xrealloc(ctl->ents,
> -						maxents * sizeof(wchar_t *));
> -			}
> -			ctl->ents[ctl->nents] = wcs;
> +			add_entry(ctl, &maxents, wcs);
>  			len = width(ctl->ents[ctl->nents]);

   len = width(wcs);

is necessary if ctl->nents will be incremented in add_entry().

>  			if (ctl->maxlength < len)
>  				ctl->maxlength = len;
 

 Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com