Hello,
Kaushal Modi <kaushal.m...@gmail.com> writes:
> On Thu, Jun 15, 2017 at 9:10 AM Kaushal Modi <kaushal.m...@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
