[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] Add an optional HOLD argument to "n" Org macro
From: |
Nicolas Goaziou |
Subject: |
Re: [O] Add an optional HOLD argument to "n" Org macro |
Date: |
Thu, 15 Jun 2017 18:07:28 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Hello,
Kaushal Modi <address@hidden> writes:
> On Thu, Jun 15, 2017 at 9:10 AM Kaushal Modi <address@hidden> 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: [O] [RFC] The "c" Org macro, Kaushal Modi, 2017/06/14
- [O] Add an optional HOLD argument to "n" Org macro (Was: [RFC] The "c" Org macro), Kaushal Modi, 2017/06/14
- Re: [O] Add an optional HOLD argument to "n" Org macro, Nicolas Goaziou, 2017/06/14
- Re: [O] Add an optional HOLD argument to "n" Org macro, Kaushal Modi, 2017/06/15
- Re: [O] Add an optional HOLD argument to "n" Org macro, Kaushal Modi, 2017/06/15
- Re: [O] Add an optional HOLD argument to "n" Org macro,
Nicolas Goaziou <=
- Re: [O] Add an optional HOLD argument to "n" Org macro, Kaushal Modi, 2017/06/15
- Re: [O] Add an optional HOLD argument to "n" Org macro, Kaushal Modi, 2017/06/17
- Re: [O] Add an optional HOLD argument to "n" Org macro, Nicolas Goaziou, 2017/06/17
- Re: [O] Add an optional HOLD argument to "n" Org macro, Kaushal Modi, 2017/06/18
- Re: [O] Add an optional HOLD argument to "n" Org macro, Nicolas Goaziou, 2017/06/18
- Re: [O] Add an optional HOLD argument to "n" Org macro, Kaushal Modi, 2017/06/18
Re: [O] [RFC] The "c" Org macro, Nicolas Goaziou, 2017/06/14