[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: patch (Re: i18n of cdebconf)

[Tomohiro KUBOTA]
> Here is the patch.  It replaces the line-folding (or text-wrapping)
> algorithm for "text", "slang", and "newt" frontends.  Please check
> it.  Or, should I cvs-commit it at first?

I read your patch, and have a few comments.  It was very small.  That
is good.  You should only declare one variable per line, to make it
easier to find changes in the future using CVS.  I would also like to
have the textwrap code as a compile time option for now, and let it
use the new library you uploaded instead of including the code in the
cdebconf source.

I suggest protecting your changes with '#ifdef HAVE_LIBTEXTWRAP', and
adding code in configure.in to detect if the library is present.  This
way, we can easily test it without enabling it in the uploaded
version.  I would like to be able to test it first, and then perhaps
remove the compile time option to make it required.  If you protect
the code like this, and keep the library code out of the cdebconf
source, feel free to commit the change. :)

Also, I want to make sure all parts of the installer use the same
shared libtextwrap, to limit the disk image size.

Regarding the library itself, I am not convinced that this charset
normalization is a good idea:

  Since the return value of nl_langinfo() may differ depending on
  systems (such as "EUC-JP" vs "eucJP"), the encoding names are
  normalized (all-uppercase, eliminate non-alphanumerics).

It is better to find and fix all systems not using a standard charmap
name.  But this will be hidden in the library, so cdebconf do not
really need to worry about it.

Reply to: