diff.c: use utf8_strwidth() instead of strlen() for display width
PATCH 1/1 diff.c: use utf8_strwidth() instead of strlen() for display width
From: Chandra Pratap <[email protected]>
---
diff.c: use utf8_strwidth() instead of strlen() for display width
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1668%2FChand-ra%2Fdiff-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1668/Chand-ra/diff-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1668
diff.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff.c +1 -6
@@ -2712,13 +2712,8 @@ 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); + width =term_columns () -utf8_strwidth (line_prefix);else width = options->stat_width ? options->stat_width :80 ; number_width = decimal_width(max_change) > number_width ?
base-commit: 2996f11c1d11ab68823f0939b6469dedc2b9ab90
Signature · 1 line
gitgitgadget
Quoted 19 lines · Chandra Pratap via GitGitGadget
From: Chandra Pratap <[email protected]>
Signed-off-by Chandra Pratap <[email protected]>---
diff.c: useutf8_strwidth()instead ofstrlen()for display widthPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1668%2FChand-ra%2Fdiff-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1668/Chand-ra/diff-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1668diff.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)diff.c +0
-0@@ -2712,13 +2712,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)* making the line longer than the maximum width.
diff.c static void show_stats(struct diffstat_t *data, struct diff_options *options) · line 2716
Quoted 10 lines
diff.c +1
-6*/ -/* -* 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); + width =term_columns () -utf8_strwidth (line_prefix);
It would be nice to add a test demonstrating that this indeed fixes an issue. This would also help to keep this from regressing in the future.
Also, do you know why we didn't use utf8_strwidth() right from the start? It would have saved the writer some time to just use utf8_strwidth() instead of writing a whole paragraph explaining that we should do it eventually. Makes me wonder whether there is anything else going on here.
Patrick
Quoted 8 lines
diff.c +0
-0else width = options->stat_width ? options->stat_width :80 ; number_width = decimal_width(max_change) > number_width ?base-commit:
2996f11c1d11ab68823f0939b6469dedc2b9ab90Signature · 1 line
gitgitgadget
Quoted 5 lines · Patrick Steinhardt
Also, do you know why we didn't use
utf8_strwidth()right from the start? It would have saved the writer some time to just useutf8_strwidth()instead of writing a whole paragraph explaining that we should do it eventually. Makes me wonder whether there is anything else going on here.
I suspect that it is because it is not just that single strlen() call. The code assumed that byte count and display width were interchangeable, and use of strlen() there was merely an example. Starting there, the value returned by strlen() is treated as if it were interchangeable with a display width, and then later used to count how many bytes to trim from either end of the string so that the trimmed string would eventually fit a given display width, which means that the code to compute how much to trim (which is not that strlen() the patch in question is touching) would have to compute the reverse, i.e. "if we need to recover N display columns, how many bytes do we need to trim from that string?"