From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kaushal Modi Subject: Re: Add an optional HOLD argument to "n" Org macro Date: Sun, 18 Jun 2017 04:03:05 +0000 Message-ID: References: <2ee94a64a94b46259b0da6e7d34675c9@HE1PR01MB1898.eurprd01.prod.exchangelabs.com> <87y3u7o3dj.fsf@t3610> <87pofjtk4b.fsf@t3610> <2069df8c23bc43f3b04b6e203b96be9d@HE1PR01MB1898.eurprd01.prod.exchangelabs.com> <87r2zvpyst.fsf@delle7240> <8760guib5i.fsf@nicolasgoaziou.fr> <87fuftb4lg.fsf@nicolasgoaziou.fr> <8760fyic6n.fsf@nicolasgoaziou.fr> <871sqli69r.fsf@nicolasgoaziou.fr> <87zid6dwop.fsf@nicolasgoaziou.fr> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="f403045fff242265f9055234199d" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:53055) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dMRQl-000311-PF for emacs-orgmode@gnu.org; Sun, 18 Jun 2017 00:03:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dMRQk-0002Ae-Oe for emacs-orgmode@gnu.org; Sun, 18 Jun 2017 00:03:19 -0400 Received: from mail-lf0-x22e.google.com ([2a00:1450:4010:c07::22e]:34595) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dMRQk-0002AY-Fh for emacs-orgmode@gnu.org; Sun, 18 Jun 2017 00:03:18 -0400 Received: by mail-lf0-x22e.google.com with SMTP id v20so40863566lfa.1 for ; Sat, 17 Jun 2017 21:03:18 -0700 (PDT) In-Reply-To: <87zid6dwop.fsf@nicolasgoaziou.fr> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Nicolas Goaziou Cc: emacs-orgmode@gnu.org --f403045fff242265f9055234199d Content-Type: text/plain; charset="UTF-8" On Sat, Jun 17, 2017 at 7:25 PM Nicolas Goaziou wrote: > Hello, > > Thank you. LGTM. I only have nitpicks. > Thanks! > > + (let ((action-trimmed (when (org-string-nw-p action) > > + (org-trim action))) > > (and (org-string-nw-p action) ...) > Ah! I forgot making this change and the patch is already pushed. > However, NAME is always a string, so it could simply be > > (name-trimmed (org-trim-name)) > Correct. I made this change. > and since you use it only once, I would simply do the suggestion below... > > > + (puthash name-trimmed > > ... which is > > (puthash (org-trim name)) > name-trimmed is then used in gethash forms too. So I retained the name-trimmed var. > You can push it whenever you think it is good enough. > Thanks. I pushed the patch, but I missed making the when -> and change. Do you want me to fix that in another commit? Though, functionally they are the same, and 'make test' is passing. -- Kaushal Modi --f403045fff242265f9055234199d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Sat, Jun 17= , 2017 at 7:25 PM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
Hello,

Thank you. LGTM. I only have nitpicks.

<= div>Thanks!
=C2=A0
> +=C2=A0 (let ((action-trimmed (when (org-string-nw-p action)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 (org-trim action)))

=C2=A0 (and (org-string-nw-p action) ...)

Ah! I forgot making this change and the patch is already pushed.
=C2=A0
However, NAME is always a str= ing, so it could simply be

=C2=A0 (name-trimmed (org-trim-name))

C= orrect. I made this change.
=C2=A0
and since you use it only once, I would simply do the suggestion belo= w...

> +=C2=A0 =C2=A0 (puthash name-trimmed

... which is

=C2=A0 =C2=A0(puthash (org-trim name))

= name-trimmed is then used in gethash forms too. So I retained the name-trim= med var.
=C2=A0
You can push = it whenever you think it is good enough.

Thanks. I pushed the patch, but I missed making the when -> and change= . Do you want me to fix that in another commit? Though, functionally they a= re the same, and 'make test' is passing.
--

Kaushal Modi

--f403045fff242265f9055234199d--