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

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

[Lennard Hofmann]

Signed-off-by: Lennard Hofmann <lennard.hofmann@web.de>
---
It surprised me that the -L option only works in table mode because I like
column(1) for columnating long terminal output on wide screens. I am not aware
of a simple way to achieve this using other Unix utilities: pr(1) truncates
columns and requires you to calculate the number of columns and the output
width manually.

The following patches remove the unnecessary restriction of the -L option. The
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.

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;
+}
+
+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);
+			}
 			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]);
 			if (ctl->maxlength < len)
 				ctl->maxlength = len;
--
2.28.0

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

[Karel Zak]

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

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

[Lennard Hofmann]

> It would be more robust to add also
> 
>     ctl->nents++;
> 
> to this function.

Agreed. I thought that would require making this ugly change

-len = width(ctl->ents[ctl->nents]);
+len = width(ctl->ents[ctl->nents - 1]);

but `len = width(wcs);` is much better!

I will send patch v2 that includes patches 1 to 3 with your suggestions added.

Lennard