Re: [PATCH 1/2] Add conversions to and from struct timespec to module interface.

[Philipp Stephani <p.stephani2@gmail.com>]

Thanks, I've addressed these comments. Since they were minor, I've
pushed the result to master as commit bffceab633.

Am Mi., 24. Apr. 2019 um 09:21 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Cc: Philipp Stephani <phst@google.com>
> > Date: Tue, 23 Apr 2019 23:32:17 +0200
> >
> > +@deftypefn Function struct timespec extract_time (emacs_env *@var{env}, emacs_value @var{time})
> > +This function, which is available since Emacs 27, interprets
> > +@var{time} as an Emacs time value and returns the corresponding
> > +@code{struct timespec}.  @xref{Time of Day}.
>
> I think we need to tell something about 'struct timespec' here.
> Ideally, simply describe its members, unless they are too platform
> dependent.  This text is for C programmers, who will need to look up
> the struct to be able to use this function.

I've documented the structure members and added a reference to the
libc manual. Fortunately the members are standardized by both C and
Posix.

> >                                                                    If
> > +@var{time} has higher precision than nanoseconds, then this function
> > +truncates it to nanosecond precision.
>
> If we describe 'struct timespec', this sentence will be redundant, I
> think.

Not really, there could be other options (e.g. signaling an error).

>  (Btw, "truncates" or "rounds", and if the former, why not the
> latter?)

I think that's just the behavior of lisp_time_argument. I'll see that
I can add further unit tests and improve the documentation.

>
> > +This function, which is available since Emacs 27, takes a @code{struct
> > +timespec} argument @var{time} and returns the corresponding Emacs
> > +timestamp.  @xref{Time of Day} for the possible return value formats.
>                                  ^
> There should be a comma there, or makeinfo will produce a warning.

I don't see any warning, and I think adding a comma there wouldn't be correct.

>
> > +It is not specified in which timestamp format the time is returned,
> > +but it is always a valid Emacs timestamp.
>
> I question the wisdom of such an obscurity.  There are no secrets
> here, as the source code is available, and I can envision use cases
> where the programmer does really need to know the structure of the
> returned timestamp.  Why should we make this "unspecified"?

To be able to change the format if we wish to do so later.
However, the current (TICKS . HZ) representation seems stable enough,
so I've documented it.

> > +@code{time.tv_nsec} doesn't have to be in the range [0, 999999999].
>                                                        ^^^^^^^^^^^^^^
> This should be in @code, and I think using @dots{} instead of the
> comma will look better in print.

This was meant to be a mathematical closed interval. Since people
might not be familiar with that notation, I've reworded it.