On Fri, Oct 21, 2022 at 02:59:09PM -0700, Junio C Hamano wrote: > Torsten Bögershausen <tboegi@xxxxxx> writes: > > > For the moment I don't have any spare time to spend on Git. > > All your comments are noted, and I hope to get time to address them later. > > If you kick out the branch from seen and the whats cooking list, > > that would be fine with me. > > I'd rather not waste the efforts so far. I am tempted to queue the > following on top or squash it in. > > ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ----- > Subject: [PATCH] diff: leave NEEDWORK notes in show_stats() function > > The previous step made an attempt to correctly compute display > columns allocated and padded different parts of diffstat output. > There are at least two known codepaths in the function that still > mixes up display widths and byte length that need to be fixed. > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > diff.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/diff.c b/diff.c > index 2751cae131..1d222d87b2 100644 > --- a/diff.c > +++ b/diff.c > @@ -2675,6 +2675,11 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) > * making the line longer than the maximum width. > */ > > + /* > + * NEEDSWORK: line_prefix is often used for "log --graph" output > + * and contains ANSI-colored string. utf8_strnwidth() should be > + * used to correctly count the display width instead of strlen(). > + */ > if (options->stat_width == -1) > width = term_columns() - strlen(line_prefix); > else > @@ -2750,6 +2755,16 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) > char *slash; > prefix = "..."; > len -= 3; > + /* > + * NEEDSWORK: (name_len - len) counts the display > + * width, which would be shorter than the byte > + * length of the corresponding substring. > + * Advancing "name" by that number of bytes does > + * *NOT* skip over that many columns, so it is > + * very likely that chomping the pathname at the > + * slash we will find starting from "name" will > + * leave the resulting string still too long. > + */ > name += name_len - len; > slash = strchr(name, '/'); > if (slash) That looks good to me - my preferred version would be a patch on it's own on top.
