bugGNU troff - Bugs: bug #51330, preconv fails to detect utf-8...

 
 

bug #51330: preconv fails to detect utf-8 without BOM

Submitted by:  Bertrand Garrigues <bgarrigues>
Submitted on:  Tue 27 Jun 2017 10:31:10 PM UTC  
 
Category: NoneSeverity: 4 - Important
Item Group: Incorrect behaviourStatus: Fixed
Privacy: PublicAssigned to: Bertrand Garrigues <bgarrigues>
Open/Closed: ClosedPlanned Release: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sat 22 Jul 2017 12:29:25 AM UTC, comment #7:

I've taken into account your last remark and commited on master. Have a nice vacation! (by the way, I'm also on vacation, starting today).

Bertrand Garrigues <bgarrigues>
Project AdministratorIn charge of this item.
Mon 17 Jul 2017 08:45:31 PM UTC, comment #6:

Sorry, I'm more or less on vacation right now and can only quickly skim your refinements...

It looks good (again :-), please commit.

Only one minor issue: Please don't talk about `libuchardet.so' in `configure.ac' and the documentation – on other platforms this library file might be called differently. It's better to talk about the `uchardet library', omitting the details how it gets linked to groff.

Werner LEMBERG <wl>
Project Administrator
Wed 12 Jul 2017 11:53:39 AM UTC, comment #5:

I've also fixed missing space after semi-colon in fprintf calls and updated the man page.

Bertrand Garrigues <bgarrigues>
Project AdministratorIn charge of this item.
Wed 12 Jul 2017 11:51:52 AM UTC, comment #4:

My previous patch was not correct, I don't remember how I tested it, but it cannot work as I have to rewind the file in function detect_file_encoding. I've made more serious tests and now I have 2 patches ready for commit.

- 0001-Add-gnulib-module-fprintf-posix.patch: this adds gnulib's `fprintf-posix' module to `bootstrap-conf'. It's somehow difficult for me to think 'pre-C99'. I had to add inclusions of 'config.h' in some files because I had errors like:

./lib/math.h:33:3: error: #error "Please include config.h first."
#error "Please include config.h first.

- 0002-Use-libchardet-for-preconv.patch: new patch that fixes the previous one. I've also fixed the style problem on single-line ELSE clause. As for the auto-detection we now have a `--with-uchardet' option that works like this (tested with and without uchardet installed):

. If `yes' is passed and uchardet is not found, configure fails.

. If `no' is passed we don't use uchardet and don't display any warning.

. If `auto' (or any other option) is passed of if --with-uchardet is not passed, we use uchardet if available, otherwise the build goes on but a warning is displayed at the end of the configuration phase.

With this patch `typesetting.pdf' is correctly generated.

(file #41186, file #41187)

Bertrand Garrigues <bgarrigues>
Project AdministratorIn charge of this item.
Wed 05 Jul 2017 04:15:48 AM UTC, comment #3:

Looks good, thanks. Some nits.

. I suggest to use an auto-detection mode for uchardet, this is, having an option

where option `auto' is the default.

. In error messages, insert a space after the colon, e.g.

. The flag `z' you are using in `fprintf' is available only in C99 – AFAIK, we don't insist on such a `modern' compiler. So if you really want to use that flag, you have to add gnulib's `fprintf-posix' module to `bootstrap-conf'.

. In groff, single-line ELSE clauses in C or C++ code are not surrounded by braces.

Werner LEMBERG <wl>
Project Administrator
Tue 04 Jul 2017 10:29:54 PM UTC, comment #2:

Yes you are right, libmagic returns a textual description that must be parsed. Uchardet is really easy to use, here is a draft patch to automatically detects if libuchardet is available with pkg-config, and use it in preconv if available. Not the definitive version though (man doc should be updated, and I also need to check which is the 1st usable version of uchardet, for the moment I put version 0.0.1).

(file #41121)

Bertrand Garrigues <bgarrigues>
Project AdministratorIn charge of this item.
Wed 28 Jun 2017 08:58:27 AM UTC, comment #1:

I like the idea of using a library to guess the charset and encoding. However, I think that libmagic is not suited to that – as far as I can see, it returns a textual description of the data that preconv had to parse manually. Please correct me if I'm wrong.

Looking around, the probably best choice is uchardet:

https://www.freedesktop.org/wiki/Software/uchardet/

We could make preconv use it optionally if it is available.

Werner LEMBERG <wl>
Project Administrator
Tue 27 Jun 2017 10:31:10 PM UTC, original submission:

(See also comment #1 from bug #50989)

typesetting.pdf (utf-8 file without BOM; contains some characters with French accents) is not correctly generated in the build tree because LC_ALL=C is passed: this causes `preconv' to use "latin1" as default encoding, which is the expected behaviour according to the man page of `preconv', and therefore characters with accents are not properly handled.

There are several quick fixes to the generation of mom examples:
- Add a BOM to the .mom files.
- Use '-K utf8' instead of just '-k'
- Add a tag to the .mom files.

However it seems to me that `preconv' should not rely on the locale to detect the file encoding.

Would it make sense to use, for example, libmagic (from the `file' utility) to make preconv correctly detect the input file encoding?

Bertrand Garrigues <bgarrigues>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by wl (Posted a comment)
  • -unavailable- added by bgarrigues (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 22 Jul 2017 12:29:39 AM UTCbgarriguesOpen/ClosedOpen=>Closed
    Sat 22 Jul 2017 12:29:25 AM UTCbgarriguesStatusReady For Test=>Fixed
    Wed 12 Jul 2017 11:51:52 AM UTCbgarriguesAttached File-=>Added 0001-Add-gnulib-module-fprintf-posix.patch, #41186
      Attached File-=>Added 0002-Use-libchardet-for-preconv.patch, #41187
      Severity3 - Normal=>4 - Important
      Item GroupNone=>Incorrect behaviour
      StatusConfirmed=>Ready For Test
    Tue 04 Jul 2017 10:29:54 PM UTCbgarriguesAttached File-=>Added 0001-Use-libchardet-for-preconv.patch, #41121

    Back to the top


    Powered by Savane 3.1-cleanup1