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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2019, 01:40 UTC
Message-ID: <xmqqbluojbzr.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.1910102303330.46@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 8 quoted lines

> This is a big heavy-handed: adding a `width` field to `struct strbuf`
> and maintaining it _just_ for the purpose of `graph.c` puts an
> unnecssary load on every other `strbuf` user (of which there are a
> _lot_).
>
> So my obvious question is: what makes `width` different from `len`?
> Since we exclusively use ASCII characters for the graph part, we should
> be able to use the already-existing `len`, for free, no?

A red-colored piece on the line consumes <RED><RESET> bytes more than the payload. Which is counted as part of "len". These bytes do not consume any display width.

When the payload is a basic CJK char in UTF-8 it may typically use 3 byte, while consuming only two display columns.

So I can understand that this application may want to keep track of <byte sequence, byte sequence length, display width> tuple.

I also understand that a programmer inexperienced/unfamiliar with our codebase may find it an easy way to satisfiy the need to add an extra field to strbuf. But as you pointed out, that is a hack unacceptable in the larger picture. Use of strbuf as "auto resizing byte array, represented as a <byte sequence, byte sequence length> tuple" is everywhere and we do not want to bloat it.

Thanks for spotting and raising this unfortunate show-stopper issue. The problem being solved is worth solving, but it needs to be done without butchering a basic data structure used elsewhere.