I've fixed a couple of corner cases that were left out in the refactoring and also changed the variables to better reflect their purpose.
I urge everyone to please try out this patch so that we can merge it into master ASAP. On Fri, Sep 12, 2014 at 3:50 PM, Darshit Shah <[email protected]> wrote: > I managed to hack together a fix for the issue. It wasn't as > straightforward as I was expecting it to be. I've attached a > preliminary version of the patch here for a review. > > Please review the patch and test it out to ensure that it works > correctly. I'll clean up the variable names and write the relevant > ChangeLog entries later and submit the final patch. > > On Thu, Sep 4, 2014 at 10:34 PM, Giuseppe Scrivano <[email protected]> wrote: >> Darshit Shah <[email protected]> writes: >> >>> Yes, I guess I'm looking for UTF-8 strings, because other character >>> encodings wouldn't create this problem, (I think?) >>> I'll look at the Wiki page again and see of GNULib has anything, Right >>> now, I'm trying to implement a solution based on wide characters >>> through wchar.h but I don't like the code I've written. I's prefer >>> something more elegant and efficient. >> >> have you had a look at the mbiter module of gnulib? Its documentation >> says that you can turn: >> >> char *iter; >> for (iter = buf; iter < buf + buflen; iter++) >> { >> do_something (*iter); >> } >> >> into: >> >> mbi_iterator_t iter; >> for (mbi_init (iter, buf, buflen); mbi_avail (iter); mbi_advance >> (iter)) >> { >> do_something (mbi_cur_ptr (iter), mb_len (mbi_cur (iter))); >> } >> >> >> and seems quite straightforward to me. >> >> Regards, >> Giuseppe > > > > -- > Thanking You, > Darshit Shah -- Thanking You, Darshit Shah
From efe090df89eb5f3b831f1483ef4c33fbae4665a2 Mon Sep 17 00:00:00 2001 From: Darshit Shah <[email protected]> Date: Fri, 12 Sep 2014 13:33:20 +0530 Subject: [PATCH] Handle multibyte characters in progressbar This commit fixes a bug in the progressbar implementation wherein filenames with multibyte characters were not handled correctly. --- ChangeLog | 4 ++++ bootstrap.conf | 1 + src/progress.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++------------ 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8b693be..c4e7809 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2014-09-12 Darshit Shah <[email protected]> + + * bootstrap.conf: Add GNULib module mbiter + 2014-07-25 Darshit Shah <[email protected]> * .gitignore: Add a gitignore file for the project. diff --git a/bootstrap.conf b/bootstrap.conf index 516bbb6..bbfb38f 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -49,6 +49,7 @@ iconv iconv-h listen maintainer-makefile +mbiter mbtowc mkdir mkstemp diff --git a/src/progress.c b/src/progress.c index e9182cc..5ba542d 100644 --- a/src/progress.c +++ b/src/progress.c @@ -37,6 +37,7 @@ as that of the covered work. */ #include <unistd.h> #include <signal.h> #include <wchar.h> +#include <mbiter.h> #include "progress.h" #include "utils.h" @@ -812,8 +813,37 @@ count_cols (const char *mbs) } return cols; } + +static int +cols_to_bytes (const char *mbs, const int cols, int *ncols) +{ + int p_cols = 0, bytes = 0; + mbchar_t mbc; + mbi_iterator_t iter; + mbi_init (iter, mbs, strlen(mbs)); + while (p_cols < cols && mbi_avail (iter)) + { + mbc = mbi_cur (iter); + p_cols += mb_width (mbc); + /* The multibyte character has exceeded the total number of columns we + * have available. The remaining bytes will be padded with a space. */ + if (p_cols > cols) + { + p_cols -= mb_width (mbc); + break; + } + bytes += mb_len (mbc); + mbi_advance (iter); + } + *ncols = p_cols; + return bytes; +} #else # define count_cols(mbs) ((int)(strlen(mbs))) +# define cols_to_bytes(mbs, cols, *ncols) do { \ + *ncols = cols; \ + bytes = cols; \ +}while (0) #endif static const char * @@ -873,7 +903,7 @@ get_eta (int *bcd) static void create_image (struct bar_progress *bp, double dl_total_time, bool done) { - const int MAX_FILENAME_LEN = bp->width / 4; + const int MAX_FILENAME_COLS = bp->width / 4; char *p = bp->buffer; wgint size = bp->initial_length + bp->count; @@ -884,7 +914,7 @@ create_image (struct bar_progress *bp, double dl_total_time, bool done) int size_grouped_pad; /* Used to pad the field width for size_grouped. */ struct bar_progress_hist *hist = &bp->hist; - int orig_filename_len = strlen (bp->f_download); + int orig_filename_cols = count_cols (bp->f_download); /* The progress bar should look like this: file xx% [=======> ] nnn.nnK 12.34KB/s eta 36m 51s @@ -896,7 +926,7 @@ create_image (struct bar_progress *bp, double dl_total_time, bool done) It would be especially bad for the progress bar to be resized randomly. - "file " - Downloaded filename - MAX_FILENAME_LEN chars + 1 + "file " - Downloaded filename - MAX_FILENAME_COLS chars + 1 "xx% " or "100%" - percentage - 4 chars "[]" - progress bar decorations - 2 chars " nnn.nnK" - downloaded bytes - 7 chars + 1 @@ -906,7 +936,7 @@ create_image (struct bar_progress *bp, double dl_total_time, bool done) "=====>..." - progress bar - the rest */ -#define PROGRESS_FILENAME_LEN MAX_FILENAME_LEN + 1 +#define PROGRESS_FILENAME_LEN MAX_FILENAME_COLS + 1 #define PROGRESS_PERCENT_LEN 4 #define PROGRESS_DECORAT_LEN 2 #define PROGRESS_FILESIZE_LEN 7 + 1 @@ -924,24 +954,31 @@ create_image (struct bar_progress *bp, double dl_total_time, bool done) if (progress_size < 5) progress_size = 0; - if (orig_filename_len <= MAX_FILENAME_LEN) + if (orig_filename_cols <= MAX_FILENAME_COLS) { - int padding = MAX_FILENAME_LEN - orig_filename_len; + int padding = MAX_FILENAME_COLS - orig_filename_cols; sprintf (p, "%s ", bp->f_download); - p += orig_filename_len + 1; + p += orig_filename_cols + 1; for (;padding;padding--) *p++ = ' '; } else { - int offset; + int offset_cols; + int bytes_in_filename, offset_bytes, col; + int *cols_ret = &col; - if (((orig_filename_len > MAX_FILENAME_LEN) && !opt.noscroll) && !done) - offset = ((int) bp->tick) % (orig_filename_len - MAX_FILENAME_LEN); + if (((orig_filename_cols > MAX_FILENAME_COLS) && !opt.noscroll) && !done) + offset_cols = ((int) bp->tick) % (orig_filename_cols - MAX_FILENAME_COLS); else - offset = 0; - memcpy (p, bp->f_download + offset, MAX_FILENAME_LEN); - p += MAX_FILENAME_LEN; + offset_cols = 0; + offset_bytes = cols_to_bytes (bp->f_download, offset_cols, cols_ret); + bytes_in_filename = cols_to_bytes (bp->f_download + offset_bytes, MAX_FILENAME_COLS, cols_ret); + memcpy (p, bp->f_download + offset_bytes, bytes_in_filename); + p += bytes_in_filename; + int padding = MAX_FILENAME_COLS - *cols_ret; + for (;padding;padding--) + *p++ = ' '; *p++ = ' '; } -- 2.1.0
