Re: [Guile-Lib PATCH v2 4/6] logging: Allow passing source properties to `log-msg'.

[Maxim Cournoyer]

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Note: this changes the API of the `accept-log' method by adding a new
> positional argument to it.  `accept-log' is a "weak" public
> interface (mostly intended for internal uses or logging handler
> implementors), so this is deemed acceptable.
>
> * src/logging/logger.scm (log-helper) [source-properties]: New
> positional argument, which is passed to `accept-log'.
> * src/logging/logger.scm (log-msg): Update doc.  Add two new variants
> that accept source properties.  Annotate more LVL arguments with their
> type to ensure proper resolution.
> (default-log-formatter) [source-properties]: New optional argument,
> that is formatted as a source location prefix when available.
> (accept-log): Update doc.  Add new source-properties argument, and
> pass it to the log-formatter procedure.
> * unit-tests/logging.logger.scm
> (test-log-with-source-properties): New test.
> * unit-tests/guile-library.api: Regenerate.
>
> ---
>
> Changes in v2:
>  - Relax log-msg typing on source-properties, as it can also be #f

[...]

Some extra thoughts on the v2 change described above:  perhaps instead
of dropping the class type from source-properties, it'd be
best to add two more define-method procedures, like so:

>  (define-method (log-msg (lvl <symbol>) . objs)
>    (if default-logger
> -      (log-helper default-logger lvl objs)))
> +      (log-helper default-logger lvl objs #f)))
> +
> +(define-method (log-msg source-properties (lvl <symbol>) . objs)
> +  (if default-logger
> +      (log-helper default-logger lvl objs source-properties)))

--8<---------------cut here---------------start------------->8---
(define-method (log-msg (source-properties <pair>) (lvl <symbol>) . objs)
  (if default-logger
      (log-helper default-logger lvl objs source-properties)))

(define-method (log-msg (source-properties <bool>) (lvl <symbol>) . objs)
  (if default-logger
      (log-helper default-logger lvl objs source-properties)))
--8<---------------cut here---------------end--------------->8---


As when using a syntactic wrapper for log-msg, source-properties would
typically be an alist, but may also be #f when running the code before
it was  byte compiled.

> +
> +(define-method (log-msg (lgr <logger>) (lvl <symbol>) . objs)
> +  (log-helper lgr lvl objs #f))
>
> -(define-method (log-msg (lgr <logger>) lvl . objs)
> -  (log-helper lgr lvl objs))
> +(define-method (log-msg (lgr <logger>) source-properties
> +                        (lvl <symbol>) . objs)
> +  (log-helper lgr lvl objs source-properties))

Likewise here, we'd need:

--8<---------------cut here---------------start------------->8---
(define-method (log-msg (lgr <logger>) (source-properties <pair>)
                        (lvl <symbol>) . objs)
  (log-helper lgr lvl objs source-properties))

(define-method (log-msg (lgr <logger>) (source-properties <pair>)
                        (lvl <symbol>) . objs)
  (log-helper lgr lvl objs source-properties))
--8<---------------cut here---------------end--------------->8---


Is that too many methods though?  It seems the resolution mechanism is
probably fast enough to not matter.

Any thoughts?

-- 
Thanks,
Maxim