On 22-06-16 12:33pm, Stefan Sperling wrote:
> On Thu, Jun 16, 2022 at 10:47:05AM +0200, Stefan Sperling wrote:
> > On Thu, Jun 16, 2022 at 10:29:20AM +0200, Stefan Sperling wrote:
> > > Such issues also exists elsewhere. Try the attached repo which contains
> > > some text from today's index page of ja.wikipedia.org. There are some
> > > rendering issues to be observed, probably because substracting view->x
> > > doesn't do the right thing in Japanese.
> > > 
> > > I'll try to come up with some fixes, but don't hesitate to take a look
> > > if you have time :)
> > 
> > This fixes horizontal scrolling of japanese text in the log view.
> 
> Better fix below. After some experimentation I came to the conclusion
> that we should really be scrolling in units of columns, rather than
> scrolling in units of wide characters.
> 

This was a good idea.

> So with the path below, the log view's x and maxx are now defined
> in terms of columns and are no longer used to index a wide character
> string. Instead, I have added a helper function which scrolls a wide
> character string. This helper is always called to ensure it behaves
> well even for view->x == 0.
> 
> This new helper could be part of format_line(), but for now I wanted
> to keep it seperate. Otherwise I would need to update all existing
> callers of format_line() in the same patch.
> 

I think that's a good idea too; it makes sense doing this in
format_line().

> With this, horizontal scrolling in the log view behaves well in both
> got.git and my ja.git test repository attached to an earlier mail in
> this thread. In Japanese you can now see one kanji being scrolled over
> per keypress, whereas in English two letters are scrolled over. This
> works as expected with the mixed Japanese/English string in ja.git.
> The 0 and $ keys still behave as expected as well.
> 
> ok?

The patch looks good to me. And works in my tests in ja.git and got.git.

> > I noticed that wcswidth() can be used instead of my hand-rolled
> > loop I added for the blame fix, so a change to reduce this loop
> > to wcswidth() is included as well.
> 
> I have dropped this part because the blame will like need more
> work after this anyway.
> 
> diff 67461ab571e3ce9157e4a0cc09e2d632cd6f3263 ed2895855606cb8f74a22520955d3c62a4ee5b47
> blob - 8a28476cd4abc2585e93d816ed8368b82925038b
> blob + a973c4a5aef87ce408322a2aedaa71ee09f7b82d
> --- tog/tog.c
> +++ tog/tog.c
> @@ -1334,6 +1334,51 @@ done:
>  	return err;
>  }
>  
> +/* Skip the leading nscroll columns of a wide character string. */
> +const struct got_error *
> +scroll_wline(wchar_t **wlinep, wchar_t *wline, int nscroll,
> +    int col_tab_align)
> +{
> +	int cols = 0;
> +	size_t wlen = wcslen(wline);
> +	int i = 0, j = 0;
> +
> +	*wlinep = wline;
> +
> +	while (i < wlen && cols < nscroll) {
> +		int width = wcwidth(wline[i]);
> +
> +		if (width == 0) {
> +			i++;
> +			continue;
> +		}
> +
> +		if (width == 1 || width == 2) {
> +			if (cols + width > nscroll)
> +				break;
> +			cols += width;
> +			i++;
> +		} else if (width == -1) {
> +			if (wline[i] == L'\t') {
> +				width = TABSIZE -
> +				    ((cols + col_tab_align) % TABSIZE);
> +			} else {
> +				width = 1;
> +				wline[i] = L'.';
> +			}
> +			if (cols + width > nscroll)
> +				break;
> +			cols += width;
> +			i++;
> +		} else
> +			return got_error_from_errno("wcwidth");
> +		j++;
> +	}
> +
> +	*wlinep = &wline[j];
> +	return NULL;
> +}
> +
>  static const struct got_error*
>  build_refs_str(char **refs_str, struct got_reflist_head *refs,
>      struct got_object_id *id, struct got_repository *repo)
> @@ -1426,7 +1471,7 @@ draw_commit(struct tog_view *view, struct got_commit_o
>  	char datebuf[12]; /* YYYY-MM-DD + SPACE + NUL */
>  	char *logmsg0 = NULL, *logmsg = NULL;
>  	char *author = NULL;
> -	wchar_t *wlogmsg = NULL, *wauthor = NULL;
> +	wchar_t *wlogmsg = NULL, *wauthor = NULL, *scrolled_wline;
>  	int author_width, logmsg_width;
>  	char *newline, *line = NULL;
>  	int col, limit;
> @@ -1514,11 +1559,12 @@ draw_commit(struct tog_view *view, struct got_commit_o
>  	err = format_line(&wlogmsg, &logmsg_width, logmsg, limit, col, 1);
>  	if (err)
>  		goto done;
> -	if (view->x < logmsg_width - 1)
> -		waddwstr(view->window, wlogmsg + view->x);
> -	else
> -		logmsg_width = 0;
> -	col += MAX(logmsg_width - view->x, 0);
> +	err = scroll_wline(&scrolled_wline, wlogmsg, view->x, col);
> +	if (err)
> +		goto done;
> +	waddwstr(view->window, scrolled_wline);
> +	logmsg_width = wcswidth(scrolled_wline, wcslen(scrolled_wline));
> +	col += MAX(logmsg_width, 0);
>  	while (col < avail) {
>  		waddch(view->window, ' ');
>  		col++;
> @@ -1781,13 +1827,13 @@ draw_commits(struct tog_view *view)
>  	if (limit <= 1)
>  		goto done;
>  
> -	/* Grow author column size if necessary. */
> +	/* Grow author column size if necessary, and set view->maxx. */
>  	entry = s->first_displayed_entry;
>  	ncommits = 0;
>  	view->maxx = 0;
>  	while (entry) {
>  		char *author, *eol, *msg, *msg0;
> -		wchar_t *wauthor;
> +		wchar_t *wauthor, *wmsg;
>  		int width;
>  		if (ncommits >= limit - 1)
>  			break;
> @@ -1809,10 +1855,14 @@ draw_commits(struct tog_view *view)
>  		while (*msg == '\n')
>  			++msg;
>  		if ((eol = strchr(msg, '\n')))
> -			view->maxx = MAX(view->maxx, eol - msg);
> -		else
> -			view->maxx = MAX(view->maxx, strlen(msg));
> +			*eol = '\0';
> +		err = format_line(&wmsg, &width, msg, INT_MAX,
> +		    date_display_cols + author_cols, 0);
> +		if (err)
> +			goto done;
> +		view->maxx = MAX(view->maxx, width);
>  		free(msg0);
> +		free(wmsg);
>  		ncommits++;
>  		entry = TAILQ_NEXT(entry, entry);
>  	}
> 

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68