[Patch, libfortran] PR 47802 Update documentation for CTIME and FDATE

Janne Blomqvist blomqvist.janne@gmail.com
Fri Mar 4 17:46:00 GMT 2011


On Fri, Mar 4, 2011 at 15:56, Tobias Burnus <burnus@net-b.de> wrote:
> On 03/01/2011 10:26 PM, Janne Blomqvist wrote:
>>
>> Ok for trunk?
>
>  @code{CTIME} converts a system time value, such as returned by
> -@code{TIME8}, to a string of the form @samp{Sat Aug 19 18:13:14 1995}.
> +@code{TIME8}, to a localized string. Unless the application has called
> +@code{setlocale}, the output will be in the default locale, of length
> +24 and of the form @samp{Sat Aug 19 18:13:14 1995}. In other locales,
> +a longer string may result.
>
>
> I would suggest to remove the first "localized" - it gives too much focus on
> the localization, which is usually not happening.

Ok.

> (Side remark: As it is a
> kind=1 string, certain localizations are not really possible or in UTF-8
> cause string-length issues.)

Well, depends on what one means with "string length"? :)  Gfortran
will get the number of bytes used right, yes, but this might not be
the same as the number of characters which might not be the same as
the column width of the string (e.g. consider the character 'ä'
represented as whatever unicode symbol corresponds to 'ä' vs. the
unicode symbol 'a' followed by the diacritic for the two dots; both
are valid UTF-8). Anyway, this part of the documentation is probably
not the right place to launch into an extended discussion of i18n and
unicode issues.

> You should mention that - in case of the subroutine version - the RESULT
> will be blank if the string does not fit.

I believe my patch already does that, see the "Arguments" paragraph.

> +default kind. It is an @code{INTENT(OUT)} argument.  If the length of
> +this variable is too short for the localized date and time string to
> +fit completely, it will be empty on procedure return.
>
>
> I think you should scratch the "localized" here and use the wording from
> ctime. Additionally, "empty" is not really a concept of a string. I think
> you should use the word "blank". (The word "empty" fits  better for the
> result variable which would be is a string of length 0.)

Ah, yes. I was searching for a suitable word myself, as empty sounded
a bit ambiguous.

> (The repetition of the ctime wording might sound superfluous, but most
> readers directly jump to the function, not knowing the ctime wording when
> looking for ftime.)
>
>  @item @emph{Return value}:
> -The current date as a string.
> +The current date and time as a localized string.  If the
> +@code{CHARACTER} variable to which the result is assigned is too
> +short, the result is truncated.
>
>
> Again, I would leave out the "localized" here - and only mention it together
> with setlocale in the Description. I am also not sure one needs the words
> about truncation as this is a general concept of string assignment.
> (Additionally, it is not quite true: Instead of truncation also a
> reallocation with the right size could happen - for allocatable "len=:"
> strings.)

Good point; I removed the notice about truncation.

> With the nits fixed - or at least considered, the patch is OK. Thanks for
> working on thread safety.

Thanks for the review and suggestions. The patch I ultimately
committed is attached.



-- 
Janne Blomqvist
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ctimedoc.diff
Type: text/x-patch
Size: 3216 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110304/5e94e483/attachment.bin>


More information about the Gcc-patches mailing list