octave-maintainers
[Top][All Lists]
Advanced

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

Re: locale support patch for [fs]sscanf function


From: John W. Eaton
Subject: Re: locale support patch for [fs]sscanf function
Date: Thu, 19 Jan 2012 10:31:53 -0500

On 19-Jan-2012, CdeMills wrote:

| 
| Jordi Gutiérrez Hermoso-2 wrote
| > 
| > 
| > Note that your test on my system fails because I don't have a fr_FR
| > locale generated. I don't know how to test this function in a
| > system-independent way. I'm thinking just getting rid of that test.
| > What do you propose?
| > 
| 
| New version. Roll back the previous one and use this one instead. I use
| "warning_with_id" instead, so the warning can be transformed into an error
| an catched. It seemed logical to use "Octave:undefined-return-values" as
| identifier.
| 
| I also gave a default value to the parameter 'loc' in the base class, to
| avoid warning messages about unusued argument. 
| 
| Changelog:
| 
| Locale support implementation in [fs]scanf functions, i.e. the ability to
| scan "1,2" and get 1.2 as results under french locale, where the decimal
| separator is ','. This uses the C++ standard library locale approach: the
| interpretation of some stream locale is modified using the "imbue" function.
| In case of fscanf, the previous locale is restored after each call. The
| functions interface is not modified. When the last argument is a string, its
| value is used as the new locale setting.
| 
| *src/oct-stream.h: Class octave_base_stream: add a virtual imbue function,
| which merely ignore its argument without generating a compile warning. Class
| octave_stream: add a real implementation, which is a proxy to
| std::[io]stream.imbue ().
| *src/file_io.cc: Function fscanf: when the last argument is a string, save
| the actual locale value and set a new value from this string. Generate a
| warning with ID Octave:undefined-return-values" in case the requested locale
| is not supported. Revert to previous locale if OK, or to standard locale in
| case of problem. Function scanf: Likewise, except the stream is generated
| on-the-fly, there is no need to restore its locale value. Added a test case
| delivering a failure only if the fr_FR locale is installed. Updated the doc
| for both functions. 

There is a prevailing style for the ChangeLog entry part of a commit
message.  Please use it, as it makes finding what was changed somewhat
easier.  There's also usually no need to go into great detail in a
ChangeLog entry.  For example, instead of the above, write a brief
one-line summary for Mercurial followed by the ChangeLog info like
this (but not indented in the actual commit message):

  handle locale [fs]scanf functions

  * src/oct-stream.h (octave_base_stream::imbue): New virtual function.
  (octave_stream::imbue): New function.
  * src/file_io.cc (Ffscanf): Handle locale, warning if requested locale
  is not available.  Update docstring. 
  (Fscanf) Likewise.  Update docstring.
  New tests.

  The "F" prefix is used for DEFUN functions because "F" is prepended to
  the function names by the DEFUN macro.


The "undefined-return-values" warning ID is currently used for
functions that do things like this:

  function [a, b, c] = f ()
    a = 1;
    c = 2;
  end

  [a,b,c] = f
  warning: f: some elements in list of return values are undefined
  a =  1
  b = [](0x0)
  c =  2

so I don't see how it is appropriate for this case.

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]