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.

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.

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?

> 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);
 	}