From: Kyle Meyer <kyle@kyleam.com> To: TEC <tecosaur@gmail.com> Cc: Bastien <bzg@gnu.org>, org-mode-email <emacs-orgmode@gnu.org> Subject: Re: [PATCH] org-plot abstractions and extension Date: Wed, 23 Dec 2020 16:55:50 GMT Message-ID: <871rfgh36z.fsf@kyleam.com> (raw) In-Reply-To: <87wnx8ud5k.fsf@gmail.com> TEC writes: > Kyle Meyer <kyle@kyleam.com> writes: > >> case is still available under the cl- prefix. If you wanted to use it >> in 73c99bf42 (org-plot.el: add utility functions for range,ticks), I >> don't see a reason not to use it now. > > I tend to use pcase over cl-case (since it's completely built in, i.e. > no (require 'cl-lib) required). Regardless of what you tend to use, you used "case" here in 73c99bf42; the minimal fix is to add a cl- prefix, and any other switch with the justification that "case is obsolete" is likely to raise a reviewer's eyebrow. cl-case isn't in cl-lib, and there is no need to load anything. > I'm not sure if there's any argument for cl-case over pcase, let me > know if so. It depends on who you ask (and, if you search emacs.devel, you will unsurprisingly see a range of opinions). I like pcase (and pattern matching in general) but think it's overused in simple cases. Here's a recent org-plot example from 8d5122fc5: (when (pcase x ('y t) ('yes t) ('t t)) ...) That could be rewritten as (when (memq x '(y yes t)) ...) As for the "[cl-]case -> pcase" change in this patch, I don't mind either cl-case or pcase here. It was the switch itself that I was commenting on. If you choose to leave it as is, that's fine. >>> @@ -210,9 +210,9 @@ values, namely regarding the range." >>> "From a the values in a TABLE of data, attempt to guess an appropriate number of ticks." >>> (let* ((row-data >>> (mapcar (lambda (row) (org--plot/values-stats >>> - (mapcar #'string-to-number (cdr row)) >>> - hard-min >>> - hard-max)) table)) >>> + (mapcar #'string-to-number (cdr row)) >>> + hard-min >>> + hard-max)) table)) >> >> Please drop this unrelated space change. > > Erm, this isn't unrelated. As the function being called changed length, > the indentation of the arguments is thus also changed. This change is in org--plot/sensible-tick-num. I don't spot any non-whitespace changes there. Git appears to agree with me: $ git show | grep '@@ -210,9' @@ -210,9 +210,9 @@ (defun org--plot/sensible-tick-num (table &optional hard-min hard-max) $ git show -w | grep '@@ -210,9' > Subject: [PATCH] org-plot.el: fix compiler warnings > > * (org--plot/values-stats): Replace `log10' with `log'. Please add a file name ("lisp/org-plot.el") to the start of the changelog entry.
next prev parent reply other threads:[~2020-12-23 16:56 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-06 5:50 TEC [not found] ` <87blijmnv9.fsf@gnu.org> [not found] ` <CAHNg_jM8sE4a6XvL5D8Gks4dQXfWhZvRBR33BDLkRgEgZ++ZGg@mail.gmail.com> 2020-09-15 3:43 ` TEC 2020-09-25 17:51 ` TEC 2020-10-17 2:12 ` TEC 2020-10-24 11:31 ` Bastien 2020-10-24 18:16 ` TEC 2020-11-21 11:49 ` ian martins 2020-12-09 2:58 ` TEC 2020-12-10 10:28 ` Bastien 2020-12-14 5:41 ` Bastien 2020-12-14 6:30 ` TEC 2020-12-14 6:56 ` Bastien 2020-12-23 5:09 ` Kyle Meyer 2020-12-23 5:10 ` TEC 2020-12-23 6:19 ` TEC 2020-12-23 7:14 ` Kyle Meyer 2020-12-23 8:38 ` TEC 2020-12-23 16:55 ` Kyle Meyer [this message] 2020-12-23 18:19 ` TEC 2020-12-24 3:09 ` Kyle Meyer
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: https://orgmode.org * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=871rfgh36z.fsf@kyleam.com \ --to=kyle@kyleam.com \ --cc=bzg@gnu.org \ --cc=emacs-orgmode@gnu.org \ --cc=tecosaur@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Org-mode mailing list This inbox may be cloned and mirrored by anyone: git clone --mirror https://orgmode.org/list/0 list/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 list list/ https://orgmode.org/list \ emacs-orgmode@gnu.org public-inbox-index list Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.yhetil.org/yhetil.emacs.orgmode nntp://news.gmane.io/gmane.emacs.orgmode AGPL code for this site: git clone https://public-inbox.org/public-inbox.git