From: Mark Jamsek Subject: Re: [rfc] tog horizontal scroll (diff & blame view) To: gameoftrees@openbsd.org Date: Thu, 16 Jun 2022 21:16:05 +1000 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 GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68