Re: master f866c85ac4e: (jsonrpc--log-event): Try and fix bug#68072

Re: master f866c85ac4e: (jsonrpc--log-event): Try and fix bug#68072

[Po Lu]

Stefan Monnier via Mailing list for Emacs changes <emacs-diffs@gnu.org>
writes:

> branch: master
> commit f866c85ac4e32df8061b285b6b44b15346994f3d
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Commit: Stefan Monnier <monnier@iro.umontreal.ca>
>
>     (jsonrpc--log-event): Try and fix bug#68072
>     
>     * lisp/jsonrpc.el (jsonrpc--log-event): Force the use of
>     `lisp-indent-function` in `pp-to-string`.
> ---
>  lisp/jsonrpc.el | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/lisp/jsonrpc.el b/lisp/jsonrpc.el
> index 3f33443f321..f0f5842a0ee 100644
> --- a/lisp/jsonrpc.el
> +++ b/lisp/jsonrpc.el
> @@ -1003,16 +1003,17 @@ of the API instead.")
>                                             (or method "")
>                                             (if id (format "[%s]" id) "")))))
>                 (msg
> -                (cond ((eq format 'full)
> -                       (format "%s%s\n" preamble (or json log-text)))
> -                      ((eq format 'short)
> -                       (format "%s%s\n" preamble (or log-text "")))
> -                      (t
> -                       (format "%s%s" preamble
> -                               (or (and foreign-message
> -                                        (concat "\n" (pp-to-string
> -                                                      foreign-message)))
> -                                   (concat log-text "\n")))))))
> +                (pcase format
> +                  ('full  (format "%s%s\n" preamble (or json log-text)))
> +                  ('short (format "%s%s\n" preamble (or log-text "")))
> +                  (_
> +                   (format "%s%s" preamble
> +                           (or (and foreign-message
> +                                    (let ((lisp-indent-function ;bug#68072
> +                                           #'lisp-indent-function))
> +                                      (concat "\n" (pp-to-string
> +                                                    foreign-message))))
> +                               (concat log-text "\n")))))))
>            (goto-char (point-max))
>            ;; XXX: could use `run-at-time' to delay server logs
>            ;; slightly to play nice with verbose servers' stderr.

Is there any substantive reason it was necessary to replace cond with
pcase?  If not, why was cond replaced with pcase?

Thanks.

Re: master f866c85ac4e: (jsonrpc--log-event): Try and fix bug#68072

[Stefan Monnier]

> Is there any substantive reason it was necessary to replace cond with
> pcase?  If not, why was cond replaced with pcase?

The reason why the `cond` expression was touched was to reduce
indentation in order to stay within 80 columns.  The reason why this
"touching" ended up changing the cond to a case is because of personal
style preferences.


        Stefan

Re: master f866c85ac4e: (jsonrpc--log-event): Try and fix bug#68072

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> Is there any substantive reason it was necessary to replace cond with
> pcase?  If not, why was cond replaced with pcase?

We have not enacted a project-wide ban on pcase, cl-lib, seq, and so on.
Contributors are as free to use them today as they were a couple of
months ago.  These are all fine libraries, have been for many years, and
it would in fact be remiss _not_ to use them.

So please let's not argue every single time that `pcase' is being used,
because we would be doing nothing else.

You are not forced to use it in your own contributions, of course.

Re: master f866c85ac4e: (jsonrpc--log-event): Try and fix bug#68072

[Po Lu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The reason why the `cond` expression was touched was to reduce
> indentation in order to stay within 80 columns.  The reason why this
> "touching" ended up changing the cond to a case is because of personal
> style preferences.

Generally, I find the most straightforward means of filling cond forms
that slightly overlap the 80 column limit to be moving the first clause
to a second line, and indenting the remainder as such.  Would that not
have worked here as well?