Re: [PATCH] [libtermdraw] Ensure that t.x tracks character widths

[Ori Bernstein <ori@xxxxxxxxxxxxxx>]

Applied, although I think we're still not unicode-ly correct. Right now,
the code seems to largely make the assumption that each codepoint takes
at least one cell.

So, it seems when we do a `put`, we'll clobber combining characters
with zero width. For example, taking the zalgo text:

	t̸̹͒̿̉̓ͫ͗̿e̻͍͇̤ͯͪ̔̑̿̈sͦ͑ͫ͒̀t̞̊͌

will format the string correctly into a buffer, and then walk through char
by char doing termdraw.putc to place the characters.

Termdraw.putc will place down the `t`, and advance by 1 character, then put
down the the combining mark, and advance by 0 characters, overwriting.  And
then overwrite with the next combining mark, and so on, until it's done with
all the zero width characters. Then it'll put down the `e`, which will clear
out the last combining mark, and repeat util it's done with the string.

I'm not sure what the nicest design to fix this would be, since being
able to seek around and plonk down a character at an (x,y) coordinate
still should be efficient, and it feels wrong to store arbitrary strings
for each cell. Might be the only way, though.

On Mon, 30 Oct 2017 00:36:19 -0400, "S. Gilles" <sgilles@xxxxxxxxxxxx> wrote:

> +			var cn = std.cellwidth(t.buf[i].chr)
> +			if cn > 1
> +				x += cn - 1
> +			;;

Minor nit: I tend to declare vars at the start of the function. Changed that
over here. Also added a comment, since adjusting the iteration var in two
places is odd enough that it could stand a bit of attention.

>  		;;
>  	;;
>  	/* restore cursor state */
> @@ -491,12 +495,13 @@ const strwidth = {t, str
>  
>  const putc = {t, c
>  	var idx
> +	var cn = std.max(0, std.cellwidth(c))
>  	
> -	if t.x >= t.width || t.y >= t.height
> +	if t.x + cn > t.width || t.y >= t.height
>  		-> void
>  	;;
>  
> -	damage(t, t.x, t.y, t.x+1, t.y+1)
> +	damage(t, t.x, t.y, t.x+cn, t.y+1)
>  	idx = t.y * t.width + t.x
>  	match c
>  	| '\r':	t.x = 0
> @@ -511,10 +516,12 @@ const putc = {t, c
>  		t.buf[idx].bg = t.bg
>  		t.buf[idx].attr = t.attr
>  
> -		if t.x > t.width
> +		if t.x + cn >= t.width
>  			t.y++ 
> +			t.x = 0
> +		else
> +			t.x += cn
>  		;;
> -		t.x = (t.x + 1) % t.width
>  	;;
>  }
>  
> -- 
> 2.14.3
> 
> 


-- 
    Ori Bernstein