Re: [RFC] The "c" Org macro

Re: [RFC] The "c" Org macro

[Eric S Fraga]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On Monday,  8 May 2017 at 11:26, Nicolas Goaziou wrote:
> Hello,
>
> I would like to scratch a long-standing itch and introduce the "c"
> macro, which is basically a way to handle multiple counters in an Org
> document. So, the following document

This sounds like a very useful macro to have.  Some suggestions:

1. I would prefer the argument to be called "COUNTER" instead of "SEED"
as the latter is more commonly associated with random number
generators.  This would make the documentation clearer potentially.

2. I wonder if a second argument could be optionally available to allow
the counter to start at an arbitrary number and/or be reset?

I would definitely be using the macro for, as an example, in preparing
exams and courseworks where I often have to manually put the numbers in.

thanks,
eric

-- 
: Eric S Fraga (0xFFFCF67D), Emacs 26.0.50, Org release_9.0.6-407-gc28ec3

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFC] The "c" Org macro

[Dushyant Juneja]

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

A very useful macro indeed!

One suggestion: can this also be made to support nested headings. For
instance:
* Part {{{c}}}
** Part {{{c}}}.{{{c}}}
* Part {{{c}}}

should export to:

* Part 1
** Part 1.1
* Part 2

I reckon this will be very useful when a paragraphs need to be broken into
several subheadings for clarity/context management.

Dushyant


On Mon, May 8, 2017 at 10:01 AM Eric S Fraga <e.fraga@ucl.ac.uk> wrote:

> On Monday,  8 May 2017 at 11:26, Nicolas Goaziou wrote:
> > Hello,
> >
> > I would like to scratch a long-standing itch and introduce the "c"
> > macro, which is basically a way to handle multiple counters in an Org
> > document. So, the following document
>
> This sounds like a very useful macro to have.  Some suggestions:
>
> 1. I would prefer the argument to be called "COUNTER" instead of "SEED"
> as the latter is more commonly associated with random number
> generators.  This would make the documentation clearer potentially.
>
> 2. I wonder if a second argument could be optionally available to allow
> the counter to start at an arbitrary number and/or be reset?
>
> I would definitely be using the macro for, as an example, in preparing
> exams and courseworks where I often have to manually put the numbers in.
>
> thanks,
> eric
>
> --
> : Eric S Fraga (0xFFFCF67D), Emacs 26.0.50, Org release_9.0.6-407-gc28ec3
>

[-- Attachment #2: Type: text/html, Size: 1905 bytes --]

Re: [RFC] The "c" Org macro

[Eric S Fraga]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On Monday,  8 May 2017 at 15:32, Dushyant Juneja wrote:
> A very useful macro indeed!
>
> One suggestion: can this also be made to support nested headings. For instance:
> * Part {{{c}}}
> ** Part {{{c}}}.{{{c}}}
> * Part {{{c}}}

I think this is what separate counters will enable but also motivates
need to be able to reset a counter (e.g. the sub-heading one in above
example if it were to be used in the second part).

-- 
: Eric S Fraga (0xFFFCF67D), Emacs 26.0.50, Org release_9.0.6-425-gf4fca1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFC] The "c" Org macro

[Robert Horn]

Eric S Fraga writes:

> On Monday,  8 May 2017 at 11:26, Nicolas Goaziou wrote:
>> Hello,
>>
>> I would like to scratch a long-standing itch and introduce the "c"
>> macro, which is basically a way to handle multiple counters in an Org
>> document. So, the following document
>
> This sounds like a very useful macro to have.  Some suggestions:
>
> 1. I would prefer the argument to be called "COUNTER" instead of "SEED"
> as the latter is more commonly associated with random number
> generators.  This would make the documentation clearer potentially.
Yes.  I was confused by this initially.
>
> 2. I wonder if a second argument could be optionally available to allow
> the counter to start at an arbitrary number and/or be reset?
>
'm frequently preparing documents that update something with lists.
Being able to start at a specific value makes that much more useful.

R Horn

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

Hello,

Eric S Fraga <e.fraga@ucl.ac.uk> writes:

> On Monday,  8 May 2017 at 15:32, Dushyant Juneja wrote:
>> A very useful macro indeed!
>>
>> One suggestion: can this also be made to support nested headings. For instance:
>> * Part {{{c}}}
>> ** Part {{{c}}}.{{{c}}}
>> * Part {{{c}}}
>
> I think this is what separate counters will enable but also motivates
> need to be able to reset a counter (e.g. the sub-heading one in above
> example if it were to be used in the second part).

Good idea.

Here is an updated patch, in which one can write

  {{{c(sub,reset)}}}
  {{{c(sub, 5)}}}

or even, for the default macro

 {{{c(,reset)}}}
 {{{c(, 99)}}}

I find the manual entry a bit awkward. Suggestions welcome.

Regards,

-- 
Nicolas Goaziou

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-macro-Implement-the-c-macro.patch --]
[-- Type: text/x-diff, Size: 4712 bytes --]

From 31397ffa1b0bf9795e2bc11568598af13db6c609 Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Mon, 8 May 2017 12:38:38 +0200
Subject: [PATCH] org-macro: Implement the "c" macro

* lisp/org-macro.el (org-macro--counter-table): New variable.
(org-macro--counter-initialize):
(org-macro--counter-increment): New functions.
(org-macro-initialize-templates): Use new functions.

* doc/org.texi (Macro replacement): Document new macro.
---
 doc/org.texi      | 10 ++++++++++
 lisp/org-macro.el | 38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/doc/org.texi b/doc/org.texi
index 312870f91..6072fcb78 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -10852,6 +10852,16 @@ This macro refers to the filename of the exported file, if any.
 This macro returns the value of property @var{PROPERTY-NAME} in current
 entry.  If @var{SEARCH-OPTION} (@pxref{Search options}) refers to a remote
 entry, it will be used instead.
+
+@item @{@{@{c@}@}@}
+@itemx @{@{@{c(@var{NAME})@}@}@}
+@itemx @{@{@{c(@var{NAME},@var{RESET})@}@}@}
+@cindex c, macro
+@cindex counter, macro
+This macro returns the number of occurrences of this macro expanded so far.
+You can use more than one counter using different @var{NAME} values.  If
+@var{RESET} is non-empty, the specified counter is reset to the value
+specified if it is a number, or 1 otherwise.
 @end table
 
 The surrounding brackets can be made invisible by setting
diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index 71e917b71..afd302932 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -36,8 +36,11 @@
 
 ;; Along with macros defined through #+MACRO: keyword, default
 ;; templates include the following hard-coded macros:
-;; {{{time(format-string)}}}, {{{property(node-property)}}},
-;; {{{input-file}}} and {{{modification-time(format-string)}}}.
+;;   {{{time(format-string)}}},
+;;   {{{property(node-property)}}},
+;;   {{{input-file}}},
+;;   {{{modification-time(format-string)}}},
+;;   {{{c(counter,reset}}}.
 
 ;; Upon exporting, "ox.el" will also provide {{{author}}}, {{{date}}},
 ;; {{{email}}} and {{{title}}} macros.
@@ -129,7 +132,7 @@ function installs the following ones: \"property\",
 	    (let ((old-template (assoc (car cell) templates)))
 	      (if old-template (setcdr old-template (cdr cell))
 		(push cell templates))))))
-    ;; Install hard-coded macros.
+    ;; Install "property", "time" macros.
     (mapc update-templates
 	  (list (cons "property"
 		      "(eval (save-excursion
@@ -143,6 +146,7 @@ function installs the following ones: \"property\",
                       l)))))
         (org-entry-get nil \"$1\" 'selective)))")
 		(cons "time" "(eval (format-time-string \"$1\"))")))
+    ;; Install "input-file", "modification-time" macros.
     (let ((visited-file (buffer-file-name (buffer-base-buffer))))
       (when (and visited-file (file-exists-p visited-file))
 	(mapc update-templates
@@ -152,6 +156,10 @@ function installs the following ones: \"property\",
 				  (prin1-to-string visited-file)
 				  (prin1-to-string
 				   (nth 5 (file-attributes visited-file)))))))))
+    ;; Initialize and install "c" macro.
+    (org-macro--counter-initialize)
+    (funcall update-templates
+	     (cons "c" "(eval (org-macro--counter-increment \"$1\" \"$2\"))"))
     (setq org-macro-templates templates)))
 
 (defun org-macro-expand (macro templates)
@@ -280,6 +288,9 @@ Return a list of arguments, as strings.  This is the opposite of
     s nil t)
    "\000"))
 
+\f
+;;; Helper functions and variables for internal macros
+
 (defun org-macro--vc-modified-time (file)
   (save-window-excursion
     (when (vc-backend file)
@@ -304,6 +315,27 @@ Return a list of arguments, as strings.  This is the opposite of
 	  (kill-buffer buf))
 	date))))
 
+(defvar org-macro--counter-table nil
+  "Hash table containing counter value per name.")
+
+(defun org-macro--counter-initialize ()
+  "Initialize `org-macro--counter-table'."
+  (setq org-macro--counter-table (make-hash-table :test #'equal)))
+
+(defun org-macro--counter-increment (name &optional reset)
+  "Increment counter NAME.
+NAME is a string identifying the counter.  If optional argument
+RESET is a non-empty string, reset the counter instead."
+  (if (org-string-nw-p reset)
+      (let ((new-value (if (string-match-p "\\`[ \t]*[0-9]+[ \t]*\\'" reset)
+			   (string-to-number reset)
+			 1)))
+	(puthash name new-value org-macro--counter-table))
+    (let ((value (gethash name org-macro--counter-table)))
+      (puthash name
+	       (if (null value) 1 (1+ value))
+	       org-macro--counter-table))))
+
 
 (provide 'org-macro)
 ;;; org-macro.el ends here
-- 
2.12.2

Re: [RFC] The "c" Org macro

[Carsten Dominik]

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

Hi,

I like the idea of these counters, and I have use-cases for them.

Here is a proposal for the manual entry

This macro implements custom counters by returning the number of times
the macro has been expanded so far while exporting the buffer.
You can create more than one counter using different @var{NAME} values.  If
@var{RESET} is non-empty, the specified counter is reset to the value
specified if it is a number, or 1 otherwise. You may leave @var{NAME}
empty to reset the default counter will be reset.

A question regarding the last sentence of this proposed manual entry:

is {{{c(,2)}}} allowed to reset the default counter?  I think is should be,
but I have
not tested this.

Carsten

On Mon, May 8, 2017 at 6:52 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Eric S Fraga <e.fraga@ucl.ac.uk> writes:
>
> > On Monday,  8 May 2017 at 15:32, Dushyant Juneja wrote:
> >> A very useful macro indeed!
> >>
> >> One suggestion: can this also be made to support nested headings. For
> instance:
> >> * Part {{{c}}}
> >> ** Part {{{c}}}.{{{c}}}
> >> * Part {{{c}}}
> >
> > I think this is what separate counters will enable but also motivates
> > need to be able to reset a counter (e.g. the sub-heading one in above
> > example if it were to be used in the second part).
>
> Good idea.
>
> Here is an updated patch, in which one can write
>
>   {{{c(sub,reset)}}}
>   {{{c(sub, 5)}}}
>
> or even, for the default macro
>
>  {{{c(,reset)}}}
>  {{{c(, 99)}}}
>
> I find the manual entry a bit awkward. Suggestions welcome.
>
> Regards,
>
> --
> Nicolas Goaziou
>

[-- Attachment #2: Type: text/html, Size: 2389 bytes --]

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

Hello,

Carsten Dominik <dominik@uva.nl> writes:

> I like the idea of these counters, and I have use-cases for them.

Great.

> Here is a proposal for the manual entry
>
> This macro implements custom counters by returning the number of times
> the macro has been expanded so far while exporting the buffer.
> You can create more than one counter using different @var{NAME} values.  If
> @var{RESET} is non-empty, the specified counter is reset to the value
> specified if it is a number, or 1 otherwise. You may leave @var{NAME}
> empty to reset the default counter will be reset.

Thank you.

I removed the last "will be reset" and updated my patch accordingly.
I don't post it, however, so as to not flood the ML.

> A question regarding the last sentence of this proposed manual entry:
>
> is {{{c(,2)}}} allowed to reset the default counter?  I think is should be,
> but I have not tested this.

It does reset the default counter, indeed. There is no other way to
reset it.

Regards,

-- 
Nicolas Goaziou                                                0x80A93738

Re: [RFC] The "c" Org macro

[Rasmus]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Eric S Fraga <e.fraga@ucl.ac.uk> writes:
>
>> On Monday,  8 May 2017 at 15:32, Dushyant Juneja wrote:
>>> A very useful macro indeed!
>>>
>>> One suggestion: can this also be made to support nested headings. For instance:
>>> * Part {{{c}}}
>>> ** Part {{{c}}}.{{{c}}}
>>> * Part {{{c}}}
>>
>> I think this is what separate counters will enable but also motivates
>> need to be able to reset a counter (e.g. the sub-heading one in above
>> example if it were to be used in the second part).
>
> Good idea.
>
> Here is an updated patch, in which one can write
>
>   {{{c(sub,reset)}}}
>   {{{c(sub, 5)}}}
>
> or even, for the default macro
>
>  {{{c(,reset)}}}
>  {{{c(, 99)}}}

Seems fine.  To me, "n" or "N" would be a better name for the macro, as
that suggests some sort of number, whereas "c" doesn’t really associated
with "counter" to me.  Perhaps it’s just because the syntax looks a lot
like the R combine command...

Are there a lot of cases where it would not be able to just configure how
heading numbers are printed in the backend?

Rasmus

-- 
May the Force be with you

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

Hello,

Rasmus <rasmus@gmx.us> writes:

> Seems fine.  To me, "n" or "N" would be a better name for the macro, as
> that suggests some sort of number, whereas "c" doesn’t really associated
> with "counter" to me.  Perhaps it’s just because the syntax looks a lot
> like the R combine command...

"n" seems better indeed. Not sure about "N"

> Are there a lot of cases where it would not be able to just configure how
> heading numbers are printed in the backend?

I don't know there are a lot of cases, but there are definitely some:

  * Definition
  * Property
  * Example 1
  * Property
  * Example 2

then you realize you want to demonstrate the second property before the
first one...

Regards,

-- 
Nicolas Goaziou

Re: [RFC] The "c" Org macro

[Eric S Fraga]

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]

On Monday,  8 May 2017 at 16:52, Nicolas Goaziou wrote:
> Here is an updated patch, in which one can write
>
>   {{{c(sub,reset)}}}
>   {{{c(sub, 5)}}}
>
> or even, for the default macro
>
>  {{{c(,reset)}}}
>  {{{c(, 99)}}}

Finally found some time to try the patch out.  All of the above work
very well.

thanks,
eric

-- 
: Eric S Fraga (0xFFFCF67D), Emacs 26.0.50, Org release_9.0.6-432-g6fee6b.dirty

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

Hello,

Eric S Fraga <e.fraga@ucl.ac.uk> writes:

> On Monday,  8 May 2017 at 16:52, Nicolas Goaziou wrote:
>> Here is an updated patch, in which one can write
>>
>>   {{{c(sub,reset)}}}
>>   {{{c(sub, 5)}}}
>>
>> or even, for the default macro
>>
>>  {{{c(,reset)}}}
>>  {{{c(, 99)}}}
>
> Finally found some time to try the patch out.  All of the above work
> very well.

Thank you for the feedback. 

Here is the last update, with tests and an ORG-NEWS entry. Noteworthy
change: the "c" macro is now the "n" macro.

If there are no objection nor additional suggestions, I will push it to
master in a couple of days.

Regards,

-- 
Nicolas Goaziou

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-macro-Implement-the-n-macro.patch --]
[-- Type: text/x-diff, Size: 9394 bytes --]

From 980b713f28596c7f6486dc1ccfa82f76de7c963d Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Mon, 8 May 2017 12:38:38 +0200
Subject: [PATCH] org-macro: Implement the "n" macro

* lisp/org-macro.el (org-macro--counter-table): New variable.
(org-macro--counter-initialize):
(org-macro--counter-increment): New functions.
(org-macro-initialize-templates): Use new functions.

* doc/org.texi (Macro replacement): Document new macro.

* testing/lisp/test-org-macro.el (test-org-macro/n):
(test-org-macro/property): New tests.
---
 etc/ORG-NEWS                   |   3 ++
 lisp/org-macro.el              |  38 +++++++++++++--
 testing/lisp/test-org-macro.el | 102 +++++++++++++++++++++++++++++++++++------
 3 files changed, 127 insertions(+), 16 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 3ca5b0553..b6110c412 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -197,6 +197,9 @@ contents, are now supported.
 This new function is meant to be used in back-ends supporting images
 as descriptions of links, a.k.a. image links.  See its docstring for
 details.
+**** New macro : ~{{{n}}}~
+This macro creates and increment multiple counters in a document.  See
+manual for details.
 **** Add global macros through ~org-export-global-macros~
 With this variable, one can define macros available for all documents.
 **** New keyword ~#+EXPORT_FILE_NAME~
diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index 71e917b71..f5ddb92e4 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -36,8 +36,11 @@
 
 ;; Along with macros defined through #+MACRO: keyword, default
 ;; templates include the following hard-coded macros:
-;; {{{time(format-string)}}}, {{{property(node-property)}}},
-;; {{{input-file}}} and {{{modification-time(format-string)}}}.
+;;   {{{time(format-string)}}},
+;;   {{{property(node-property)}}},
+;;   {{{input-file}}},
+;;   {{{modification-time(format-string)}}},
+;;   {{{n(counter,reset}}}.
 
 ;; Upon exporting, "ox.el" will also provide {{{author}}}, {{{date}}},
 ;; {{{email}}} and {{{title}}} macros.
@@ -129,7 +132,7 @@ function installs the following ones: \"property\",
 	    (let ((old-template (assoc (car cell) templates)))
 	      (if old-template (setcdr old-template (cdr cell))
 		(push cell templates))))))
-    ;; Install hard-coded macros.
+    ;; Install "property", "time" macros.
     (mapc update-templates
 	  (list (cons "property"
 		      "(eval (save-excursion
@@ -143,6 +146,7 @@ function installs the following ones: \"property\",
                       l)))))
         (org-entry-get nil \"$1\" 'selective)))")
 		(cons "time" "(eval (format-time-string \"$1\"))")))
+    ;; Install "input-file", "modification-time" macros.
     (let ((visited-file (buffer-file-name (buffer-base-buffer))))
       (when (and visited-file (file-exists-p visited-file))
 	(mapc update-templates
@@ -152,6 +156,10 @@ function installs the following ones: \"property\",
 				  (prin1-to-string visited-file)
 				  (prin1-to-string
 				   (nth 5 (file-attributes visited-file)))))))))
+    ;; Initialize and install "n" macro.
+    (org-macro--counter-initialize)
+    (funcall update-templates
+	     (cons "n" "(eval (org-macro--counter-increment \"$1\" \"$2\"))"))
     (setq org-macro-templates templates)))
 
 (defun org-macro-expand (macro templates)
@@ -280,6 +288,9 @@ Return a list of arguments, as strings.  This is the opposite of
     s nil t)
    "\000"))
 
+\f
+;;; Helper functions and variables for internal macros
+
 (defun org-macro--vc-modified-time (file)
   (save-window-excursion
     (when (vc-backend file)
@@ -304,6 +315,27 @@ Return a list of arguments, as strings.  This is the opposite of
 	  (kill-buffer buf))
 	date))))
 
+(defvar org-macro--counter-table nil
+  "Hash table containing counter value per name.")
+
+(defun org-macro--counter-initialize ()
+  "Initialize `org-macro--counter-table'."
+  (setq org-macro--counter-table (make-hash-table :test #'equal)))
+
+(defun org-macro--counter-increment (name &optional reset)
+  "Increment counter NAME.
+NAME is a string identifying the counter.  If optional argument
+RESET is a non-empty string, reset the counter instead."
+  (if (org-string-nw-p reset)
+      (let ((new-value (if (string-match-p "\\`[ \t]*[0-9]+[ \t]*\\'" reset)
+			   (string-to-number reset)
+			 1)))
+	(puthash name new-value org-macro--counter-table))
+    (let ((value (gethash name org-macro--counter-table)))
+      (puthash name
+	       (if (null value) 1 (1+ value))
+	       org-macro--counter-table))))
+
 
 (provide 'org-macro)
 ;;; org-macro.el ends here
diff --git a/testing/lisp/test-org-macro.el b/testing/lisp/test-org-macro.el
index 26c56745c..64b0a97cc 100644
--- a/testing/lisp/test-org-macro.el
+++ b/testing/lisp/test-org-macro.el
@@ -75,9 +75,22 @@
       (org-macro-initialize-templates)
       (org-macro-replace-all org-macro-templates)
       (buffer-string))))
-  ;; Test special "property" macro.  With only one argument, retrieve
-  ;; property from current headline.  Otherwise, the second argument
-  ;; is a search option to get the property from another headline.
+  ;; Macro expansion ignores narrowing.
+  (should
+   (string-match
+    "expansion"
+    (org-test-with-temp-text
+	"#+MACRO: macro expansion\n{{{macro}}}\n<point>Contents"
+      (narrow-to-region (point) (point-max))
+      (org-macro-initialize-templates)
+      (org-macro-replace-all org-macro-templates)
+      (org-with-wide-buffer (buffer-string))))))
+
+(ert-deftest test-org-macro/property ()
+  "Test {{{property}}} macro."
+  ;; With only one argument, retrieve property from current headline.
+  ;; Otherwise, the second argument is a search option to get the
+  ;; property from another headline.
   (should
    (equal "1"
 	  (org-test-with-temp-text
@@ -107,17 +120,80 @@
    (org-test-with-temp-text
        "* H1\n:PROPERTIES:\n:A: 1\n:END:\n* H2\n{{{property(A,*???)}}}<point>"
      (org-macro-initialize-templates)
-     (org-macro-replace-all org-macro-templates)))
-  ;; Macro expansion ignores narrowing.
+     (org-macro-replace-all org-macro-templates))))
+
+(ert-deftest test-org-macro/n ()
+  "Test {{{n}}} macro."
+  ;; Standard test with default counter.
   (should
-   (string-match
-    "expansion"
-    (org-test-with-temp-text
-	"#+MACRO: macro expansion\n{{{macro}}}\n<point>Contents"
-      (narrow-to-region (point) (point-max))
-      (org-macro-initialize-templates)
-      (org-macro-replace-all org-macro-templates)
-      (org-with-wide-buffer (buffer-string))))))
+   (equal "1 2"
+	  (org-test-with-temp-text "{{{n}}} {{{n}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position)))))
+  (should
+   (equal "1 2"
+	  (org-test-with-temp-text "{{{n()}}} {{{n}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position)))))
+  ;; Test alternative counters.
+  (should
+   (equal "1 1 1 2"
+	  (org-test-with-temp-text "{{{n}}} {{{n(c1)}}} {{{n(c2)}}} {{{n(c1)}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position)))))
+  ;; Second argument set a counter to a given value.  A non-numeric
+  ;; value resets the counter to 1.
+  (should
+   (equal "9 10"
+	  (org-test-with-temp-text "{{{n(c,9)}}} {{{n(c)}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position)))))
+  (should
+   (equal "9 1"
+	  (org-test-with-temp-text "{{{n(c,9)}}} {{{n(c,reset)}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position)))))
+  ;; Tolerate spaces in second argument.
+  (should
+   (equal "9 10"
+	  (org-test-with-temp-text "{{{n(c, 9)}}} {{{n(c)}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position)))))
+  (should
+   (equal "9 1"
+	  (org-test-with-temp-text "{{{n(c,9)}}} {{{n(c, reset)}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position)))))
+  ;; Second argument also applies to default counter.
+  (should
+   (equal "9 10 1"
+	  (org-test-with-temp-text "{{{n(,9)}}} {{{n}}} {{{n(,reset)}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position)))))
+  ;; An empty second argument is equivalent to no argument.
+  (should
+   (equal "2 3"
+	  (org-test-with-temp-text "{{{n(c,2)}}} {{{n(c,)}}}"
+	    (org-macro-initialize-templates)
+	    (org-macro-replace-all org-macro-templates)
+	    (buffer-substring-no-properties
+	     (line-beginning-position) (line-end-position))))))
 
 (ert-deftest test-org-macro/escape-arguments ()
   "Test `org-macro-escape-arguments' specifications."
-- 
2.13.0

Re: [RFC] The "c" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 2147 bytes --]

On Sun, May 21, 2017 at 9:38 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Here is the last update, with tests and an ORG-NEWS entry. Noteworthy
> change: the "c" macro is now the "n" macro.
>
> If there are no objection nor additional suggestions, I will push it to
> master in a couple of days.
>

Thanks. I tried it out and it works great, except for one issue I came
across.

If the n macro is at the BOL, followed by a period, the counter value
resets to 1 even when that macro is specifying the counter value.

Here is the full MWE:

#+TITLE: ={{{n}}}= macro

* This works
Some text /plus/ counter on the same line, no trailing period.

Foo {{{n(a, 7463)}}}
* This works
Counter at BOL, no trailing period.

Foo
{{{n(a, 7463)}}}
* This works
Some text /plus/ counter on the same line, *with* trailing period.

Foo {{{n(a, 7463)}}}.
* This *does not* work
Counter at BOL, *with* trailing period.

Foo
{{{n(a, 7463)}}}.
** How I discovered this scenario
I ended up with this scenario because of this example I was playing
with:
#+BEGIN_SRC org
# -*- fill-column: 70; eval: (auto-fill-mode 1) -*-
The counter starts by default at the value of {{{n}}}. This will be
the next value {{{n}}}, and this will be next {{{n}}}... Now this will
back to the reset value: {{{n(, reset)}}}.

If I wish, I can set that counter value to anything I want, like 7463:
{{{n(, 7463)}}}.

Here is how I can keep multiple counters counting independent of each
other. Each column is running an independent counter. The counter in
first column is initialized to 123, the one in second column to 456,
and the one in third column to 789.. and then they keep on incrementing
the count by 1 in their respective columns:

| n1               | n2               | n3               |
|------------------+------------------+------------------|
| {{{n(n1, 123)}}} | {{{n(n2, 456)}}} | {{{n(n3, 789)}}} |
| {{{n(n1)}}}      | {{{n(n2)}}}      | {{{n(n3)}}}      |
| {{{n(n1)}}}      | {{{n(n2)}}}      | {{{n(n3)}}}      |
| {{{n(n1)}}}      | {{{n(n2)}}}      | {{{n(n3)}}}      |
| {{{n(n1)}}}      | {{{n(n2)}}}      | {{{n(n3)}}}      |
#+END_SRC

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 3153 bytes --]

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

Hello,

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

> Thanks. I tried it out and it works great, except for one issue I came
> across.

Thank you for the feedback.

> If the n macro is at the BOL, followed by a period, the counter value
> resets to 1 even when that macro is specifying the counter value.
> Foo
> {{{n(a, 7463)}}}.

Even though it is surprising, this is not an issue. The thing is macros
are expanded at the very beginning of the export process, even before
the document is parsed. At this point, the example above is properly
expanded to

  7463.

However, this is then parsed as the first numbered item of a plain list,
which is then exported as

  1.

IOW, there is not much we can do. You may want to insert a zero-width
space between the macro and the dot.

Regards,

-- 
Nicolas Goaziou                                                0x80A93738

Re: [RFC] The "c" Org macro

[Eric S Fraga]

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]

On Sunday, 21 May 2017 at 13:37, Nicolas Goaziou wrote:
> If there are no objection nor additional suggestions, I will push it to
> master in a couple of days.

Looks good to me!  Thanks.
-- 
: Eric S Fraga (0xFFFCF67D), Emacs 26.0.50, Org release_9.0.7-468-g2b44a1.dirty

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFC] The "c" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

On Mon, May 22, 2017, 1:58 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Even though it is surprising, this is not an issue. The thing is macros
> are expanded at the very beginning of the export process, even before
> the document is parsed. At this point, the example above is properly
> expanded to
>
>   7463.
>
> However, this is then parsed as the first numbered item of a plain list,
> which is then exported as
>
>   1.
>

Thanks for the explanation. That does make sense. The MWE was a little
example I wanted to document for myself for this new feature. So this
example doesn't show a real world use of that macro.

IOW, there is not much we can do. You may want to insert a zero-width
> space between the macro and the dot.
>

It just so happened that autofilling ended up with a number followed by a
period at the BOL. So I couldn't have planned inserting a zero-width space
before the dot.

So this makes me think of the use of auto-filling in my regular org
files... What if auto-filling happened to create a case like this where a
line began with a number immediately followed by a period?! Though ending
sentences with a number is rare, I just need to be cautious about this.

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1956 bytes --]

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

Hello,

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

> It just so happened that autofilling ended up with a number followed by a
> period at the BOL. So I couldn't have planned inserting a zero-width space
> before the dot.

This shouldn't happen. "1." is defined as paragraph separator (see
`org-element-paragraph-separate' and `org-element--set-regexps) so
auto-fill cannot break a line before this construct.

> So this makes me think of the use of auto-filling in my regular org
> files... What if auto-filling happened to create a case like this where a
> line began with a number immediately followed by a period?!

Per above, this should not happen. There are cases, however, that can
happen, like the macro in your ECM. Auto-fill cannot tell this is going
to make a numbered bullet.


Regards,

-- 
Nicolas Goaziou                                                0x80A93738

Re: [RFC] The "c" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]

On Mon, May 22, 2017 at 7:47 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> This shouldn't happen. "1." is defined as paragraph separator (see
> `org-element-paragraph-separate' and `org-element--set-regexps) so
> auto-fill cannot break a line before this construct.
>

That's good to know and comforting :) Thanks for the explanation.


> Per above, this should not happen. There are cases, however, that can
> happen, like the macro in your ECM. Auto-fill cannot tell this is going
> to make a numbered bullet.
>

I just tried this out and it works:

diff --git a/lisp/org-element.el b/lisp/org-element.el
index c60a56ead..4f4fc1e2c 100644
--- a/lisp/org-element.el
+++ b/lisp/org-element.el
@@ -181,7 +181,9 @@ specially in `org-element--object-lex'.")
       (?\) ")") (?. "\\.") (_ "[.)]")))
       (alpha (and org-list-allow-alphabetical "\\|[A-Za-z]")))
   (concat "\\(?:[-+*]\\|\\(?:[0-9]+" alpha "\\)" term "\\)"
-  "\\(?:[ \t]\\|$\\)"))
+  "\\(?:[ \t]\\|$\\)")) "\\|"
+                ;; n Macro
+                "\\(?:{{{n\\)" ;Don't allow auto-fill to put n macros at
BOL
  "\\)\\)")
  org-element--object-regexp
  (mapconcat #'identity

With that, now the MWE auto-fills as

=====
If I wish, I can set that counter value to anything I want, like
7463: {{{n(, 7463)}}}.
=====

instead of

=====
If I wish, I can set that counter value to anything I want, like 7463:
{{{n(, 7463)}}}.
=====

and that fixes this problem.


-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 3106 bytes --]

Re: [RFC] The "c" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 681 bytes --]

This prevents false n-macro matches.. added \\b after "{{{n".

diff --git a/lisp/org-element.el b/lisp/org-element.el
index c60a56ead..2bee85ede 100644
--- a/lisp/org-element.el
+++ b/lisp/org-element.el
@@ -181,7 +181,9 @@ specially in `org-element--object-lex'.")
       (?\) ")") (?. "\\.") (_ "[.)]")))
       (alpha (and org-list-allow-alphabetical "\\|[A-Za-z]")))
   (concat "\\(?:[-+*]\\|\\(?:[0-9]+" alpha "\\)" term "\\)"
-  "\\(?:[ \t]\\|$\\)"))
+  "\\(?:[ \t]\\|$\\)")) "\\|"
+                ;; n Macro
+                "\\(?:{{{n\\b\\)" ;Don't allow auto-fill to put n macros
at BOL
  "\\)\\)")
  org-element--object-regexp
  (mapconcat #'identity

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1649 bytes --]

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

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

> I just tried this out and it works:
>
> diff --git a/lisp/org-element.el b/lisp/org-element.el
> index c60a56ead..4f4fc1e2c 100644
> --- a/lisp/org-element.el
> +++ b/lisp/org-element.el
> @@ -181,7 +181,9 @@ specially in `org-element--object-lex'.")
>        (?\) ")") (?. "\\.") (_ "[.)]")))
>        (alpha (and org-list-allow-alphabetical "\\|[A-Za-z]")))
>    (concat "\\(?:[-+*]\\|\\(?:[0-9]+" alpha "\\)" term "\\)"
> -  "\\(?:[ \t]\\|$\\)"))
> +  "\\(?:[ \t]\\|$\\)")) "\\|"
> +                ;; n Macro
> +                "\\(?:{{{n\\)" ;Don't allow auto-fill to put n macros at

This is a wrong approach. A macro doesn't separate paragraphs, but
belongs to them. IOW, you get the desired side-effect, but break
underlying syntax.

A better way to solve this would to add a function to
`fill-nobreak-predicate', like we already do for
`org-fill-line-break-nobreak-p' or
`org-fill-paragraph-with-timestamp-nobreak-p'.

I will add it to the "n" macro patch.

Thank you.

Regards,

Re: [RFC] The "c" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

On Mon, May 22, 2017 at 9:13 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> This is a wrong approach. A macro doesn't separate paragraphs, but
> belongs to them. IOW, you get the desired side-effect, but break
> underlying syntax.
>

I agree

A better way to solve this would to add a function to
> `fill-nobreak-predicate', like we already do for
> `org-fill-line-break-nobreak-p' or
> `org-fill-paragraph-with-timestamp-nobreak-p'.
>
> I will add it to the "n" macro patch.
>

I didn't know of those. Thanks!
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1098 bytes --]

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Here is the last update, with tests and an ORG-NEWS entry. Noteworthy
> change: the "c" macro is now the "n" macro.
>
> If there are no objection nor additional suggestions, I will push it to
> master in a couple of days.

Pushed.

Re: [RFC] The "c" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 173 bytes --]

On Thu, May 25, 2017 at 6:42 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

>
> Pushed.


Works great!  Thanks for fixing the auto-filling issue too.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 525 bytes --]

Re: [RFC] The "c" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

Hi Nicolas,

Can you please add the n macro information to the Org manual.

Looks like this commit (
http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=ad89390219d0de46fb9364ed067b1e9e2cf88a9b
)
missed the hunks from org.texi? (though the commit message mentions
org.texi).

Thanks.

On Thu, May 25, 2017 at 2:31 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> On Thu, May 25, 2017 at 6:42 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
> wrote:
>
>>
>> Pushed.
>
>
> Works great!  Thanks for fixing the auto-filling issue too.
> --
>
> Kaushal Modi
>
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1434 bytes --]

Add an optional HOLD argument to "n" Org macro (Was: [RFC] The "c" Org macro)

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 3256 bytes --]

I was writing something along the lines of this when I needed to specify
the n macro to hold its previous value and not increment at all.

=====
* Week {{{n(wk)}}}: Python Basics
# 1
** {{{n}}}. Introduction to Python
# 1
** {{{n}}}. Core Elements of Programs
# 2
*** {{{n(,,hold)}}}.{{{n(topic,reset)}}}: Bindings
# 2.1
*** {{{n(,,hold)}}}.{{{n(topic)}}}: Strings
# 2.2
*** {{{n(,,hold)}}}.{{{n(topic)}}}: Input/Output
# 2.3
* Week {{{n(wk)}}}: Simple Programs
# 2
** {{{n}}}. Simple Algorithms
# 3
** {{{n}}}. Functions
# 4
* Week {{{n(wk)}}}: Structured Types
# 3
** {{{n}}}. Tuples and Lists
# 5
** {{{n}}}. Dictionaries
# 6
=====

Below is an informal diff to show how the hold feature was implemented.

diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index 338c98811d..cc74cebd0a 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -40,7 +40,7 @@
 ;;   {{{property(node-property)}}},
 ;;   {{{input-file}}},
 ;;   {{{modification-time(format-string)}}},
-;;   {{{n(counter,reset}}}.
+;;   {{{n(counter,reset,hold}}}.

 ;; Upon exporting, "ox.el" will also provide {{{author}}}, {{{date}}},
 ;; {{{email}}} and {{{title}}} macros.
@@ -163,7 +163,7 @@ function installs the following ones: \"property\",
     ;; Initialize and install "n" macro.
     (org-macro--counter-initialize)
     (funcall update-templates
-     (cons "n" "(eval (org-macro--counter-increment \"$1\" \"$2\"))"))
+     (cons "n" "(eval (org-macro--counter-increment \"$1\" \"$2\"
\"$3\"))"))
     (setq org-macro-templates templates)))

 (defun org-macro-expand (macro templates)
@@ -327,14 +327,20 @@ Return a list of arguments, as strings.  This is the
opposite of
   "Initialize `org-macro--counter-table'."
   (setq org-macro--counter-table (make-hash-table :test #'equal)))

-(defun org-macro--counter-increment (name &optional reset)
+(defun org-macro--counter-increment (name &optional reset hold)
   "Increment counter NAME.
-NAME is a string identifying the counter.  When non-nil, optional
-argument RESET is a string.  If it represents an integer, set the
-counter to this number.  Any other non-empty string resets the
-counter to 1."
+NAME is a string identifying the counter.
+
+When non-nil, optional argument RESET is a string.  If it represents
+an integer, set the counter to this number.  Any other non-empty
+string resets the counter to 1.
+
+When non-nil, optional argument HOLD is a string.  If it is a
+non-empty string, keep the NAME counter at its previous value."
   (puthash name
-   (cond ((not (org-string-nw-p reset))
+   (cond ((org-string-nw-p hold)
+  (gethash name org-macro--counter-table 0))
+ ((not (org-string-nw-p reset))
   (1+ (gethash name org-macro--counter-table 0)))
  ((string-match-p "\\`[ \t]*[0-9]+[ \t]*\\'" reset)
   (string-to-number reset))


I haven't yet created a proper formatted patch, because I don't know if
this is the best implementation. Would it be better to rename the RESET
option to something more generic, like ARG? That way,

- If ARG is a "-" string, the counter value will be held and not
incremented.
- If ARG is a number represented as a string, it set the NAME counter to
that value (like now).
- Else if ARG is a non-empty string, it resets the NAME counter to 1 (like
now).

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 5822 bytes --]

Re: [RFC] The "c" Org macro

[Nicolas Goaziou]

Hello,

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

> Can you please add the n macro information to the Org manual.
>
> Looks like this commit (
> http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=ad89390219d0de46fb9364ed067b1e9e2cf88a9b
> )
> missed the hunks from org.texi? (though the commit message mentions
> org.texi).

Huh? Where did it go?

Done. Thank you.

Regards,

-- 
Nicolas Goaziou

Re: Add an optional HOLD argument to "n" Org macro

[Nicolas Goaziou]

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

> I was writing something along the lines of this when I needed to specify
> the n macro to hold its previous value and not increment at all.

[...]

> I haven't yet created a proper formatted patch, because I don't know if
> this is the best implementation. Would it be better to rename the RESET
> option to something more generic, like ARG? That way,
>
> - If ARG is a "-" string, the counter value will be held and not
> incremented.
> - If ARG is a number represented as a string, it set the NAME counter to
> that value (like now).
> - Else if ARG is a non-empty string, it resets the NAME counter to 1 (like
> now).

I think it is better since resetting, setting and holding are mutually
exclusive. With tests, please! :)

Regards,

Re: Add an optional HOLD argument to "n" Org macro

[Kaushal Modi]

[-- Attachment #1.1: Type: text/plain, Size: 301 bytes --]

On Wed, Jun 14, 2017 at 3:47 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> I think it is better since resetting, setting and holding are mutually
> exclusive.


Thanks.


> With tests, please! :)
>

Of course :)

The patch based off latest master is attached. Please review.
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 823 bytes --]

[-- Attachment #2: 0001-Add-hold-action-to-the-n-macro.patch --]
[-- Type: application/octet-stream, Size: 5224 bytes --]

From 6e6462f6f0756e1b349ae450dce4783255287c77 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Wed, 14 Jun 2017 14:34:53 -0400
Subject: [PATCH] Add hold 'action' to the "n" macro

* lisp/org-macro.el (org-macro--counter-increment): Rename the
optional arg RESET to ACTION, as now that action can mean setting,
resetting or even holding the specified counter.  ACTION set to
"hold" or "-" will hold the previous value of the counter.

* doc/org.texi (Macro replacement): Document the new hold action.

* testing/lisp/test-org-macro.el (test-org-macro/n): Add new test for
the hold action.
---
 doc/org.texi                   | 12 +++++++-----
 lisp/org-macro.el              | 26 +++++++++++++++++---------
 testing/lisp/test-org-macro.el | 17 ++++++++++++++++-
 3 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/doc/org.texi b/doc/org.texi
index 96aede2aa1..32cc1431b4 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -10867,15 +10867,17 @@ entry, that will be used instead.
 
 @item @{@{@{n@}@}@}
 @itemx @{@{@{n(@var{NAME})@}@}@}
-@itemx @{@{@{n(@var{NAME},@var{RESET})@}@}@}
+@itemx @{@{@{n(@var{NAME},@var{ACTION})@}@}@}
 @cindex n, macro
 @cindex counter, macro
 This macro implements custom counters by returning the number of times the
 macro has been expanded so far while exporting the buffer.  You can create
-more than one counter using different @var{NAME} values.  If @var{RESET} is
-non-empty, the specified counter is reset to the value specified if it is
-a number, or 1 otherwise.  You may leave @var{NAME} empty to reset the
-default counter.
+more than one counter using different @var{NAME} values.  If @var{ACTION} is
+@code{hold} or @code{-}, previous value of the counter is held, i.e. the
+specified counter is not incremented.  If the value is a number, the
+specified counter is set to that value.  If it is any other non-empty string,
+the specified counter is reset to 1.  You may leave @var{NAME} empty to reset
+the default counter.
 @end table
 
 The surrounding brackets can be made invisible by setting
diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index 338c98811d..1b3ee28f2d 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -40,7 +40,7 @@
 ;;   {{{property(node-property)}}},
 ;;   {{{input-file}}},
 ;;   {{{modification-time(format-string)}}},
-;;   {{{n(counter,reset}}}.
+;;   {{{n(counter,action}}}.
 
 ;; Upon exporting, "ox.el" will also provide {{{author}}}, {{{date}}},
 ;; {{{email}}} and {{{title}}} macros.
@@ -327,17 +327,25 @@ Return a list of arguments, as strings.  This is the opposite of
   "Initialize `org-macro--counter-table'."
   (setq org-macro--counter-table (make-hash-table :test #'equal)))
 
-(defun org-macro--counter-increment (name &optional reset)
+(defun org-macro--counter-increment (name &optional action)
   "Increment counter NAME.
-NAME is a string identifying the counter.  When non-nil, optional
-argument RESET is a string.  If it represents an integer, set the
-counter to this number.  Any other non-empty string resets the
-counter to 1."
+NAME is a string identifying the counter.
+
+When non-nil, optional argument ACTION is a string.
+
+If the string is \"hold\" or \"-\", 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."
   (puthash name
-	   (cond ((not (org-string-nw-p reset))
+	   (cond ((not (org-string-nw-p action))
 		  (1+ (gethash name org-macro--counter-table 0)))
-		 ((string-match-p "\\`[ \t]*[0-9]+[ \t]*\\'" reset)
-		  (string-to-number reset))
+		 ((string-match-p "\\`\\(-\\|hold\\)\\'" action)
+		  (gethash name org-macro--counter-table 1))
+		 ((string-match-p "\\`[ \t]*[0-9]+[ \t]*\\'" action)
+		  (string-to-number action))
 		 (t 1))
 	   org-macro--counter-table))
 
diff --git a/testing/lisp/test-org-macro.el b/testing/lisp/test-org-macro.el
index 7356e98f52..eb96140899 100644
--- a/testing/lisp/test-org-macro.el
+++ b/testing/lisp/test-org-macro.el
@@ -210,7 +210,22 @@
             (org-macro-initialize-templates)
             (org-macro-replace-all org-macro-templates)
             (buffer-substring-no-properties
-             (line-beginning-position) (line-end-position))))))
+             (line-beginning-position) (line-end-position)))))
+  ;; Second argument set to "-" or "hold" holds the counter value.
+  (should
+   (equal "1.1 2.2 8.3 8.1 8.2 9.2 9.2"
+          (org-test-with-temp-text
+	   (concat "{{{n(,-)}}}.{{{n(c)}}}" ;Hold before even starting the counter
+		   " {{{n}}}.{{{n(c)}}}"    ;Increment after hold
+		   " {{{n(,8)}}}.{{{n(c)}}}"
+		   " {{{n(,hold)}}}.{{{n(c,reset)}}}" ;Alternative hold arg
+		   " {{{n(,-)}}}.{{{n(c)}}}"
+		   " {{{n}}}.{{{n(c,hold)}}}" ;Hold on another counter
+		   " {{{n(,hold)}}}.{{{n(c,-)}}}") ;Hold on both counters
+           (org-macro-initialize-templates)
+           (org-macro-replace-all org-macro-templates)
+           (buffer-substring-no-properties
+            (line-beginning-position) (line-end-position))))))
 
 (ert-deftest test-org-macro/escape-arguments ()
   "Test `org-macro-escape-arguments' specifications."
-- 
2.13.0

Re: Add an optional HOLD argument to "n" Org macro

[Kaushal Modi]

[-- Attachment #1.1: Type: text/plain, Size: 350 bytes --]

On Thu, Jun 15, 2017 at 9:10 AM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> The patch based off latest master is attached. Please review.
>

Here's the same patch with one fix.. support white space around the
"-"/"hold" arg.

This patch adds a dependency on subr-x library for string-trim function
that was added in emacs 24.4.
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 817 bytes --]

[-- Attachment #2: 0001-Add-hold-action-to-the-n-macro.patch --]
[-- Type: application/octet-stream, Size: 5887 bytes --]

From c9275729659d403bd8f4a5921e1517c653eb96f6 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Wed, 14 Jun 2017 14:34:53 -0400
Subject: [PATCH] Add hold 'action' to the "n" macro

* lisp/org-macro.el (org-macro--counter-increment): Rename the
optional arg RESET to ACTION, as now that action can mean setting,
resetting or even holding the specified counter.  ACTION set to
"hold" or "-" will hold the previous value of the counter.

* doc/org.texi (Macro replacement): Document the new hold action.

* testing/lisp/test-org-macro.el (test-org-macro/n): Add new test for
the hold action.
---
 doc/org.texi                   | 12 +++++++-----
 lisp/org-macro.el              | 38 +++++++++++++++++++++++++-------------
 testing/lisp/test-org-macro.el | 18 +++++++++++++++++-
 3 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/doc/org.texi b/doc/org.texi
index 96aede2aa1..32cc1431b4 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -10867,15 +10867,17 @@ entry, that will be used instead.
 
 @item @{@{@{n@}@}@}
 @itemx @{@{@{n(@var{NAME})@}@}@}
-@itemx @{@{@{n(@var{NAME},@var{RESET})@}@}@}
+@itemx @{@{@{n(@var{NAME},@var{ACTION})@}@}@}
 @cindex n, macro
 @cindex counter, macro
 This macro implements custom counters by returning the number of times the
 macro has been expanded so far while exporting the buffer.  You can create
-more than one counter using different @var{NAME} values.  If @var{RESET} is
-non-empty, the specified counter is reset to the value specified if it is
-a number, or 1 otherwise.  You may leave @var{NAME} empty to reset the
-default counter.
+more than one counter using different @var{NAME} values.  If @var{ACTION} is
+@code{hold} or @code{-}, previous value of the counter is held, i.e. the
+specified counter is not incremented.  If the value is a number, the
+specified counter is set to that value.  If it is any other non-empty string,
+the specified counter is reset to 1.  You may leave @var{NAME} empty to reset
+the default counter.
 @end table
 
 The surrounding brackets can be made invisible by setting
diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index 338c98811d..cdee7474d6 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -40,7 +40,7 @@
 ;;   {{{property(node-property)}}},
 ;;   {{{input-file}}},
 ;;   {{{modification-time(format-string)}}},
-;;   {{{n(counter,reset}}}.
+;;   {{{n(counter,action}}}.
 
 ;; Upon exporting, "ox.el" will also provide {{{author}}}, {{{date}}},
 ;; {{{email}}} and {{{title}}} macros.
@@ -63,6 +63,7 @@
 (declare-function vc-backend "vc-hooks" (f))
 (declare-function vc-call "vc-hooks" (fun file &rest args) t)
 (declare-function vc-exec-after "vc-dispatcher" (code))
+(declare-function string-trim "subr-x"  (string &optional trim-left trim-right))
 
 ;;; Variables
 
@@ -327,19 +328,30 @@ Return a list of arguments, as strings.  This is the opposite of
   "Initialize `org-macro--counter-table'."
   (setq org-macro--counter-table (make-hash-table :test #'equal)))
 
-(defun org-macro--counter-increment (name &optional reset)
+(defun org-macro--counter-increment (name &optional action)
   "Increment counter NAME.
-NAME is a string identifying the counter.  When non-nil, optional
-argument RESET is a string.  If it represents an integer, set the
-counter to this number.  Any other non-empty string resets the
-counter to 1."
-  (puthash name
-	   (cond ((not (org-string-nw-p reset))
-		  (1+ (gethash name org-macro--counter-table 0)))
-		 ((string-match-p "\\`[ \t]*[0-9]+[ \t]*\\'" reset)
-		  (string-to-number reset))
-		 (t 1))
-	   org-macro--counter-table))
+NAME is a string identifying the counter.
+
+When non-nil, optional argument ACTION is a string.
+
+If the string is \"hold\" or \"-\", 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)
+			  (require 'subr-x)
+			  (string-trim action))))
+    (puthash name
+	     (cond ((not (org-string-nw-p action-trimmed))
+		    (1+ (gethash name org-macro--counter-table 0)))
+		   ((string-match-p "\\`\\(-\\|hold\\)\\'" action-trimmed)
+		    (gethash name org-macro--counter-table 1))
+		   ((string-match-p "\\`[0-9]+\\'" action-trimmed)
+		    (string-to-number action-trimmed))
+		   (t 1))
+	     org-macro--counter-table)))
 
 
 (provide 'org-macro)
diff --git a/testing/lisp/test-org-macro.el b/testing/lisp/test-org-macro.el
index 7356e98f52..84f092ed44 100644
--- a/testing/lisp/test-org-macro.el
+++ b/testing/lisp/test-org-macro.el
@@ -210,7 +210,23 @@
             (org-macro-initialize-templates)
             (org-macro-replace-all org-macro-templates)
             (buffer-substring-no-properties
-             (line-beginning-position) (line-end-position))))))
+             (line-beginning-position) (line-end-position)))))
+  ;; Second argument set to "-" or "hold" holds the counter value.
+  (should
+   (equal "1.1 2.2 8.3 8.1 8.2 8.3 9.3 9.3"
+          (org-test-with-temp-text
+	   (concat "{{{n(,-)}}}.{{{n(c)}}}" ;Hold before even starting the counter
+		   " {{{n}}}.{{{n(c)}}}"    ;Increment after hold
+		   " {{{n(,8)}}}.{{{n(c)}}}"
+		   " {{{n(,hold)}}}.{{{n(c,reset)}}}" ;Alternative hold arg
+		   " {{{n(, - )}}}.{{{n(c)}}}"	      ;With spaces
+		   " {{{n(, hold )}}}.{{{n(c)}}}"     ;With spaces
+		   " {{{n}}}.{{{n(c,hold)}}}" ;Hold on another counter
+		   " {{{n(,hold)}}}.{{{n(c,-)}}}") ;Hold on both counters
+           (org-macro-initialize-templates)
+           (org-macro-replace-all org-macro-templates)
+           (buffer-substring-no-properties
+            (line-beginning-position) (line-end-position))))))
 
 (ert-deftest test-org-macro/escape-arguments ()
   "Test `org-macro-escape-arguments' specifications."
-- 
2.13.0

Re: Add an optional HOLD argument to "n" Org macro

[Nicolas Goaziou]

Hello,

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

> On Thu, Jun 15, 2017 at 9:10 AM Kaushal Modi <kaushal.modi@gmail.com> wrote:
>
>> The patch based off latest master is attached. Please review.

Thank you. Some comments follow.

> This patch adds a dependency on subr-x library for string-trim function
> that was added in emacs 24.4.

We do not need this dependency. In particular, there is already
`org-trim'.

> * lisp/org-macro.el (org-macro--counter-increment): Rename the
> optional arg RESET to ACTION, as now that action can mean setting,
> resetting or even holding the specified counter.  ACTION set to
> "hold" or "-" will hold the previous value of the counter.

It is confusing to provide two ways to achieve the same action. I'd
rather have "-" only.

> +Any other non-empty string resets the counter to 1."
> +  (let ((action-trimmed (when (org-string-nw-p action)
> +			  (require 'subr-x)
> +			  (string-trim action))))

See above.

> +  ;; Second argument set to "-" or "hold" holds the counter value.
> +  (should
> +   (equal "1.1 2.2 8.3 8.1 8.2 8.3 9.3 9.3"
> +          (org-test-with-temp-text
> +	   (concat "{{{n(,-)}}}.{{{n(c)}}}" ;Hold before even starting the counter
> +		   " {{{n}}}.{{{n(c)}}}"    ;Increment after hold
> +		   " {{{n(,8)}}}.{{{n(c)}}}"
> +		   " {{{n(,hold)}}}.{{{n(c,reset)}}}" ;Alternative hold arg
> +		   " {{{n(, - )}}}.{{{n(c)}}}"	      ;With spaces
> +		   " {{{n(, hold )}}}.{{{n(c)}}}"     ;With spaces
> +		   " {{{n}}}.{{{n(c,hold)}}}" ;Hold on another counter
> +		   " {{{n(,hold)}}}.{{{n(c,-)}}}") ;Hold on both counters
> +           (org-macro-initialize-templates)
> +           (org-macro-replace-all org-macro-templates)
> +           (buffer-substring-no-properties
> +            (line-beginning-position) (line-end-position))))))

Could you split this into smaller tests, each one testing one feature?

Regards,

-- 
Nicolas Goaziou

Re: Add an optional HOLD argument to "n" Org macro

[Kaushal Modi]

[-- Attachment #1.1: Type: text/plain, Size: 659 bytes --]

On Thu, Jun 15, 2017 at 12:07 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> We do not need this dependency. In particular, there is already
> `org-trim'.
>

OK, switch from string-trim to org-trim. Thanks.


> It is confusing to provide two ways to achieve the same action. I'd
> rather have "-" only.
>

OK.


> Could you split this into smaller tests, each one testing one feature?
>

Done. I ended up writing many more tests in the process. While I was doing
that, I realized that only the first arg NAME was ws (whitespace) sensitive
while the second arg wasn't. Now ws trimming is done on both args.

Revised patch is attached.
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 1359 bytes --]

[-- Attachment #2: 0001-Add-hold-action-to-the-n-macro-and-ws-trim-all-n-mac.patch --]
[-- Type: application/octet-stream, Size: 9420 bytes --]

From 0f96645aaf72d68fd7096e4135ab06e554296f46 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Thu, 15 Jun 2017 14:00:07 -0400
Subject: [PATCH] Add hold 'action' to the "n" macro and ws-trim all "n" macro
 args

* lisp/org-macro.el (org-macro--counter-increment): Rename the
optional arg RESET to ACTION, as now that action can mean setting,
resetting or even holding the specified counter.  ACTION set to
"-" will hold the previous value of the counter.  White-space is
now trimmed from the NAME arg too.

* doc/org.texi (Macro replacement): Document the new hold action.

* testing/lisp/test-org-macro.el (test-org-macro/n): Add new tests for
the hold action.
---
 doc/org.texi                   | 11 +++---
 lisp/org-macro.el              | 38 +++++++++++-------
 testing/lisp/test-org-macro.el | 87 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 113 insertions(+), 23 deletions(-)

diff --git a/doc/org.texi b/doc/org.texi
index 5ba7d06bbc..110510a230 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -10867,15 +10867,16 @@ entry, that will be used instead.
 
 @item @{@{@{n@}@}@}
 @itemx @{@{@{n(@var{NAME})@}@}@}
-@itemx @{@{@{n(@var{NAME},@var{RESET})@}@}@}
+@itemx @{@{@{n(@var{NAME},@var{ACTION})@}@}@}
 @cindex n, macro
 @cindex counter, macro
 This macro implements custom counters by returning the number of times the
 macro has been expanded so far while exporting the buffer.  You can create
-more than one counter using different @var{NAME} values.  If @var{RESET} is
-non-empty, the specified counter is reset to the value specified if it is
-a number, or 1 otherwise.  You may leave @var{NAME} empty to reset the
-default counter.
+more than one counter using different @var{NAME} values.  If @var{ACTION} is
+@code{-}, previous value of the counter is held, i.e. the specified counter
+is not incremented.  If the value is a number, the specified counter is set
+to that value.  If it is any other non-empty string, the specified counter is
+reset to 1.  You may leave @var{NAME} empty to reset the default counter.
 @end table
 
 The surrounding brackets can be made invisible by setting
diff --git a/lisp/org-macro.el b/lisp/org-macro.el
index 338c98811d..d18bf60bc7 100644
--- a/lisp/org-macro.el
+++ b/lisp/org-macro.el
@@ -40,7 +40,7 @@
 ;;   {{{property(node-property)}}},
 ;;   {{{input-file}}},
 ;;   {{{modification-time(format-string)}}},
-;;   {{{n(counter,reset}}}.
+;;   {{{n(counter,action}}}.
 
 ;; Upon exporting, "ox.el" will also provide {{{author}}}, {{{date}}},
 ;; {{{email}}} and {{{title}}} macros.
@@ -327,19 +327,31 @@ Return a list of arguments, as strings.  This is the opposite of
   "Initialize `org-macro--counter-table'."
   (setq org-macro--counter-table (make-hash-table :test #'equal)))
 
-(defun org-macro--counter-increment (name &optional reset)
+(defun org-macro--counter-increment (name &optional action)
   "Increment counter NAME.
-NAME is a string identifying the counter.  When non-nil, optional
-argument RESET is a string.  If it represents an integer, set the
-counter to this number.  Any other non-empty string resets the
-counter to 1."
-  (puthash name
-	   (cond ((not (org-string-nw-p reset))
-		  (1+ (gethash name org-macro--counter-table 0)))
-		 ((string-match-p "\\`[ \t]*[0-9]+[ \t]*\\'" reset)
-		  (string-to-number reset))
-		 (t 1))
-	   org-macro--counter-table))
+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)))
+        (name-trimmed (when (org-string-nw-p name)
+                        (org-trim name))))
+    (puthash name-trimmed
+             (cond ((not (org-string-nw-p action-trimmed))
+                    (1+ (gethash name-trimmed org-macro--counter-table 0)))
+                   ((string= "-" action-trimmed)
+                    (gethash name-trimmed org-macro--counter-table 1))
+                   ((string-match-p "\\`[0-9]+\\'" action-trimmed)
+                    (string-to-number action-trimmed))
+                   (t 1))
+             org-macro--counter-table)))
 
 
 (provide 'org-macro)
diff --git a/testing/lisp/test-org-macro.el b/testing/lisp/test-org-macro.el
index 7356e98f52..664bc88dfb 100644
--- a/testing/lisp/test-org-macro.el
+++ b/testing/lisp/test-org-macro.el
@@ -180,17 +180,51 @@
             (org-macro-replace-all org-macro-templates)
             (buffer-substring-no-properties
              (line-beginning-position) (line-end-position)))))
-  ;; Tolerate spaces in second argument.
+  ;; Check that reset happens when the second argument is neither "-"
+  ;; nor a number.
   (should
-   (equal "9 10"
-          (org-test-with-temp-text "{{{n(c, 9)}}} {{{n(c)}}}"
+   (equal "9 1 1 1"
+          (org-test-with-temp-text
+	      (concat "{{{n(c,9)}}} {{{n(c,reiniciar)}}}"
+		      " {{{n(c,réinitialiser)}}} {{{n(c,zurückstellen)}}}")
             (org-macro-initialize-templates)
             (org-macro-replace-all org-macro-templates)
             (buffer-substring-no-properties
              (line-beginning-position) (line-end-position)))))
+  ;; Tolerate spaces in first argument.
   (should
-   (equal "9 1"
-          (org-test-with-temp-text "{{{n(c,9)}}} {{{n(c, reset)}}}"
+   (equal "1 2 3 4"
+          (org-test-with-temp-text "{{{n(c)}}} {{{n(c )}}} {{{n( c)}}} {{{n( c )}}}"
+            (org-macro-initialize-templates)
+            (org-macro-replace-all org-macro-templates)
+            (buffer-substring-no-properties
+             (line-beginning-position) (line-end-position)))))
+  ;; Tolerate spaces when second argument is an integer.
+  (should
+   (equal "2 3 5 7"
+          (org-test-with-temp-text
+	      (concat "{{{n(c,2)}}} {{{n(c, 3)}}}"
+		      " {{{n(c,5 )}}} {{{n(c, 7 )}}}")
+            (org-macro-initialize-templates)
+            (org-macro-replace-all org-macro-templates)
+            (buffer-substring-no-properties
+             (line-beginning-position) (line-end-position)))))
+  ;; Tolerate spaces when second argument is the hold argument.
+  (should
+   (equal "7 7 8 8 9 9"
+          (org-test-with-temp-text
+	      (concat "{{{n(,7)}}} {{{n(, -)}}}"
+		      " {{{n}}} {{{n(,- )}}} {{{n}}} {{{n(, - )}}}")
+            (org-macro-initialize-templates)
+            (org-macro-replace-all org-macro-templates)
+            (buffer-substring-no-properties
+             (line-beginning-position) (line-end-position)))))
+  ;; Tolerate spaces when second argument is used to reset the counter.
+  (should
+   (equal "9 1 1 1 1"
+          (org-test-with-temp-text
+	      (concat "{{{n(c,9)}}} {{{n(c,reset)}}} {{{n(c, reset)}}}"
+		      " {{{n(c,reset )}}} {{{n(c, reset )}}}")
             (org-macro-initialize-templates)
             (org-macro-replace-all org-macro-templates)
             (buffer-substring-no-properties
@@ -209,6 +243,49 @@
           (org-test-with-temp-text "{{{n(c,2)}}} {{{n(c,)}}}"
             (org-macro-initialize-templates)
             (org-macro-replace-all org-macro-templates)
+            (buffer-substring-no-properties
+             (line-beginning-position) (line-end-position)))))
+  ;; Hold value at reset value of 1 if the counter hasn't yet started.
+  (should
+   (equal "1"
+          (org-test-with-temp-text "{{{n(,-)}}}"
+            (org-macro-initialize-templates)
+            (org-macro-replace-all org-macro-templates)
+            (buffer-substring-no-properties
+             (line-beginning-position) (line-end-position)))))
+  ;; Increment counter following a hold.
+  (should
+   (equal "1 1 2"
+          (org-test-with-temp-text "{{{n}}} {{{n(,-)}}} {{{n}}}"
+            (org-macro-initialize-templates)
+            (org-macro-replace-all org-macro-templates)
+            (buffer-substring-no-properties
+             (line-beginning-position) (line-end-position)))))
+  ;; Hold counter value following a counter value set.
+  (should
+   (equal "1 10 10"
+          (org-test-with-temp-text "{{{n}}} {{{n(,10)}}} {{{n(,-)}}}"
+            (org-macro-initialize-templates)
+            (org-macro-replace-all org-macro-templates)
+            (buffer-substring-no-properties
+             (line-beginning-position) (line-end-position)))))
+  ;; Hold counter value in a multiple-counter situation.
+  (should
+   (equal "1.1 1.2 1.3"
+          (org-test-with-temp-text
+	      "{{{n}}}.{{{n(c)}}} {{{n(,-)}}}.{{{n(c)}}} {{{n(,-)}}}.{{{n(c)}}}"
+            (org-macro-initialize-templates)
+            (org-macro-replace-all org-macro-templates)
+            (buffer-substring-no-properties
+             (line-beginning-position) (line-end-position)))))
+  ;; Hold counter values on one or multiple counters at the same time.
+  (should
+   (equal "1.1 1.2 2.2 2.2"
+          (org-test-with-temp-text
+	      (concat "{{{n}}}.{{{n(c)}}} {{{n(,-)}}}.{{{n(c)}}}"
+		      " {{{n}}}.{{{n(c,-)}}} {{{n(,-)}}}.{{{n(c,-)}}}")
+            (org-macro-initialize-templates)
+            (org-macro-replace-all org-macro-templates)
             (buffer-substring-no-properties
              (line-beginning-position) (line-end-position))))))
 
-- 
2.13.0

Re: Add an optional HOLD argument to "n" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

Hello Nicolas,

Just pinging to check if the patch in the previous email got missed.

Apologies if you were already reviewing it.

Thanks.

On Thu, Jun 15, 2017, 2:07 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> On Thu, Jun 15, 2017 at 12:07 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
> wrote:
>
>> We do not need this dependency. In particular, there is already
>> `org-trim'.
>>
>
> OK, switch from string-trim to org-trim. Thanks.
>
>
>> It is confusing to provide two ways to achieve the same action. I'd
>> rather have "-" only.
>>
>
> OK.
>
>
>> Could you split this into smaller tests, each one testing one feature?
>>
>
> Done. I ended up writing many more tests in the process. While I was doing
> that, I realized that only the first arg NAME was ws (whitespace) sensitive
> while the second arg wasn't. Now ws trimming is done on both args.
>
> Revised patch is attached.
> --
>
> Kaushal Modi
>
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2257 bytes --]

Re: Add an optional HOLD argument to "n" Org macro

[Nicolas Goaziou]

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

Re: Add an optional HOLD argument to "n" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

On Sat, Jun 17, 2017 at 7:25 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
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

[-- Attachment #2: Type: text/html, Size: 2039 bytes --]

Re: Add an optional HOLD argument to "n" Org macro

[Nicolas Goaziou]

Hello,

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

> 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.

As I said, I was only nitpicking. Feel free to fix it, or not.

Regards,

-- 
Nicolas Goaziou

Re: Add an optional HOLD argument to "n" Org macro

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 243 bytes --]

Hello Nicolas,

On Sun, Jun 18, 2017, 3:16 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

>
> As I said, I was only nitpicking. Feel free to fix it, or not.
>

OK, thanks for confirming. I'll leave out this edit then.

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 770 bytes --]