Re: [Bug-wget] patch: Improve the rolling file name length for downloading progress image when without NLS

[Tim Ruehsen]

On Friday, February 17, 2017 12:11:54 AM CET YX Hao wrote:
> Dear Tim,
> 
> 
> 
> I think you may misunderstand my situation or just starting to reproduce the
> issue.
> 
> My configuration result is NO NLS support, that means 'USE_NLS_PROGRESS_BAR'
> is undefined.

Hi YX Hao,

@Andries please read further down

The point is that defining USE_NLS_PROGRESS_BAR is dependent on the existence 
of wcwidth() and mbtowc() which has no meaning if you build wget with or 
without NLS.
In other words: Having those functions and building with NLS are two different 
things. To prove that:

$ ./configure --help|grep -i nls
  --disable-nls           do not use Native Language Support

$ ./configure --enable-nls
$ grep WCWIDTH src/config.h
#define HAVE_WCWIDTH 1
$ grep NLS src/config.h
#define ENABLE_NLS 1

$ ./configure --disable-nls
$ grep WCWIDTH src/config.h
#define HAVE_WCWIDTH 1
$ grep NLS src/config.h
/* #undef ENABLE_NLS */

So we can completely get rid of USE_NLS_PROGRESS_BAR, since it is a bug 
anyways.

This has nothing (or just partly) to do with your issue, we just have to fix it 
before we fix something at the wrong place.


> I guess screenshot is necessary at this time.
> 
> 
> 
> And it will always fall to the 'else' case of
> 'progress-cols_to_bytes-err.png'. And this 'cols_to_bytes' function is the
> problem. It is called by 'create_image' function as
> 'progress-create_image-caller.png'. Got a more than actual bytes to copy,
> the wrong 'bytes_in_filename', when the leftover is less than
> 'MAX_FILENAME_COLS - padding'.
> 
> This will contains the file names tailing '\0' and some random value bytes.
> Plus, wrong 'bytes_in_filename' won't get the '\0' to be filled by blank
> char '\20'. '\0' will short the future 'print'.
> 
> >I would like to get rid of USE_NLS_PROGRESS_BAR.
> >
> >It is a shortcut for
> >
> >        HAVE_WCWIDTH && HAVE_MBTOWC
> >
> >gnulib provides us with these functions on systems, where they don't exist
> 
> in
> 
> >the standard C library.
> >
> >
> >
> >For a quick check, could you unconditionally set USE_NLS_PROGRESS_BARin
> 
> wget.h
> 
> >(lines 78 and 80-83 should be commented), recompile wget and report if it
> >
> >works for you ?
> 
> I have done it. Got 'NO_ USE_NLS_PROGRESS_BAR.png'. Just to make sure that
> we are on the same page.

This image clearly shows that you forgot to comment line 82 (the #undef).
So you define USE_NLS_PROGRESS_BAR and then undefine it ;-) 


Calculating the number of displayed columns from the number of bytes of a 
string is non-trivial. It is trivial only for charsets/locales where each byte 
(or codepoint) will take exactly one column on the display.

With unicode you have to *at least* compose the string first (NFC I guess), and 
then count the codepoints. But I am not sure about exceptions.

@Andries Do you know an algorithm how to calculate the columns from a given 
string + encoding ?

Regards, Tim

signature.asc
Description: This is a digitally signed message part.