From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Subject: Re: Add an optional HOLD argument to "n" Org macro
Date: Sun, 18 Jun 2017 01:24:54 +0200
Message-ID: <87zid6dwop.fsf@nicolasgoaziou.fr>
References: <2ee94a64a94b46259b0da6e7d34675c9@HE1PR01MB1898.eurprd01.prod.exchangelabs.com>
	<87y3u7o3dj.fsf@t3610>
	<a4c6d561b12a4cc8ad4fe8c017fa2121@HE1PR01MB1898.eurprd01.prod.exchangelabs.com>
	<87pofjtk4b.fsf@t3610>
	<2069df8c23bc43f3b04b6e203b96be9d@HE1PR01MB1898.eurprd01.prod.exchangelabs.com>
	<87r2zvpyst.fsf@delle7240> <8760guib5i.fsf@nicolasgoaziou.fr>
	<87fuftb4lg.fsf@nicolasgoaziou.fr>
	<CAFyQvY1cLtMXFgNh0R-Fv_hg+8TWGEGe_NKLebPykk=rkevCiw@mail.gmail.com>
	<CAFyQvY0+rLM+kKTqHGNuMnL9asmafnLYqLJ97UOGqLaXuFmbDw@mail.gmail.com>
	<CAFyQvY19R4gEkEP1pBwvVguyC081u2qn5WuDHkC_mGv_S_5YrQ@mail.gmail.com>
	<8760fyic6n.fsf@nicolasgoaziou.fr>
	<CAFyQvY133N0k1K3A_Gc2b9fP4MrRMcco=WwzayD3mG6R==BALg@mail.gmail.com>
	<CAFyQvY3EmjqoFBYu_PkiGTJoNzt_ZW7pwRCTVCMZh24MSNxJQQ@mail.gmail.com>
	<871sqli69r.fsf@nicolasgoaziou.fr>
	<CAFyQvY3FaNXNmYpp92TPy1ZkBwKNfPPjM1+JMSmvxRC1jQcYzQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain
Return-path: <emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:54460)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mail@nicolasgoaziou.fr>) id 1dMN5R-0002vK-N3
	for emacs-orgmode@gnu.org; Sat, 17 Jun 2017 19:25:02 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mail@nicolasgoaziou.fr>) id 1dMN5O-0007Kp-Jn
	for emacs-orgmode@gnu.org; Sat, 17 Jun 2017 19:25:01 -0400
Received: from relay4-d.mail.gandi.net ([217.70.183.196]:48187)
	by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)
	(Exim 4.71) (envelope-from <mail@nicolasgoaziou.fr>)
	id 1dMN5O-0007K5-DV
	for emacs-orgmode@gnu.org; Sat, 17 Jun 2017 19:24:58 -0400
In-Reply-To: <CAFyQvY3FaNXNmYpp92TPy1ZkBwKNfPPjM1+JMSmvxRC1jQcYzQ@mail.gmail.com>
	(Kaushal Modi's message of "Thu, 15 Jun 2017 18:07:19 +0000")
List-Id: "General discussions about Org-mode." <emacs-orgmode.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/emacs-orgmode/>
List-Post: <mailto:emacs-orgmode@gnu.org>
List-Help: <mailto:emacs-orgmode-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=subscribe>
Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org
Sender: "Emacs-orgmode"
	<emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org>
To: Kaushal Modi <kaushal.modi@gmail.com>
Cc: emacs-orgmode@gnu.org

Hello,

Kaushal Modi <kaushal.modi@gmail.com> writes:

> Revised patch is attached.

Thank you. LGTM. I only have nitpicks.

> +NAME is a string identifying the counter.
> +
> +When non-nil, optional argument ACTION is a string.
> +
> +If the string is \"-\", keep the NAME counter at its current
> +value, i.e. do not increment.
> +
> +If the string represents an integer, set the counter to this number.
> +
> +Any other non-empty string resets the counter to 1."

> +  (let ((action-trimmed (when (org-string-nw-p action)
> +                          (org-trim action)))

  (and (org-string-nw-p action) ...)

> +        (name-trimmed (when (org-string-nw-p name)
> +                        (org-trim name))))

  (and (org-string-nw-p name) ...)

However, NAME is always a string, so it could simply be

  (name-trimmed (org-trim-name))

and since you use it only once, I would simply do the suggestion below...

> +    (puthash name-trimmed

... which is

   (puthash (org-trim name))

You can push it whenever you think it is good enough.

Regards,

-- 
Nicolas Goaziou                                                0x80A93738