Re: [PATCH 01/11] graph: automatically track visible width of `strbuf`

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Oct 11, 2019, 16:02 UTC
Message-ID: <nycvar.QRO.7.76.6.1910111800220.46@tvgsbejvaqbjf.bet>
In-Reply-To: <20191011050111.GA94866@generichostname>

Hi,

On Thu, 10 Oct 2019, Denton Liu wrote:

Show 30 quoted lines

> On Fri, Oct 11, 2019 at 10:42:20AM +0900, Junio C Hamano wrote:
> > Denton Liu <liu.denton@gmail.com> writes:
> >
> > > 	static int calculate_width(const struct strbuf *row)
> > > 	{
> > > 		int in_termcode = 0;
> > > 		int width = 0;
> > > 		int i;
> > >
> > > 		for (i = 0; i < row.len; i++) {
> > > 			if (row.buf[i] == '\033')
> > > 				in_termcode = 1;
> > >
> > > 			if (!in_termcode)
> > > 				width++;
> > > 			else if (row.buf[i] == 'm')
> > > 				in_termcode = 0;
> > > 		}
> > > 	}
> >
> > Not every byte that is outside the escape sequence contributes to
> > one display columns.  You would want to take a look at utf8_width()
> > for inspiration.
> >
>
> Heh, I guess you're right. Looking right below the definition of
> utf8_width, I realised we have the utf8_strnwidth function. We should be
> able to just call
>
> 	utf8_strnwidth(row.buf, row.len, 1);

Correct me if I'm wrong, but don't we want to keep track of the columns *only* in the part with the actual line graph, i.e. we're not at all interested in the onelines' widths?

If so, I could imagine that a good idea would be to introduce

	struct graphbuf {
		struct strbuf buf;
		int width;
	};

and then introduce wrappers for `_addch()` and whatever else is used in `graph.c`, these wrappers will increment the width together with the `buf.len` field, and one additional helper that adds color sequences to that graphbuf that leaves `width` unchanged.

Ciao, Dscho