Org campture recursively expands %-escapes

Org campture recursively expands %-escapes

[Thomas Preindl]

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

Hi everyone,

setting up my capture templates to work with a new Chrome extension I
noticed that when i mark some text containing %-escapes inserted with the
'%i' in the template the %-escape was
evaluated.

For example, marking %(print (buffer-name)) will be replaced with
"*Capture*".

I am now wondering if this is intended or not and if this could be
used as a kind of exploit to run code if someone captures code
from a website.

Is there a way to prevent this? I thought about escaping the string, but I
would have to change the chrome extension or maybe is it possible to escape
it somehow in the template?

Here is my template:
("p" "org-protocol-Ch-marked" entry (file refile-path)
         "* %:description\n  %U\n  %:link\n  #+BEGIN_QUOTE\n  %i\n
 #+END_QUOTE"  :immediate-finish t :empty-lines-after 1)

br,
Thomas

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

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Hello,

Thomas Preindl <thomas.preindl@gmail.com> writes:

> setting up my capture templates to work with a new Chrome extension I
> noticed that when i mark some text containing %-escapes inserted with the
> '%i' in the template the %-escape was
> evaluated.
>
> For example, marking %(print (buffer-name)) will be replaced with
> "*Capture*".
>
> I am now wondering if this is intended or not and if this could be
> used as a kind of exploit to run code if someone captures code
> from a website.

Judging from `org-capture-fill-template', this is a feature. Worse,
%(...) placeholders, the most dangerous ones, are always expanded last.

I guess the intent is to fill the Sexp with previous placeholders and
then eval it for a proper result (see, e.g., `org-capture-template's
docstring).

One solution would be to expand recursively Sexp placeholders at the
beginning of `org-capture-fill-template', right after expanding
properties placeholders (i.e., %:property), so as to limit the problem.

We could also remove recursivity for placeholders altogether. It is
buggy anyway (e.g., if a property placeholder introduces another
placeholder, the latter is not expanded).

Question to the ML: is there anyone relying on placeholder recursion?


Regards,

-- 
Nicolas Goaziou

Re: Org campture recursively expands %-escapes

[Samuel Wales]

just to clarify,

  "%(alpha-org-protocol-string \"%:link\" \"%:description\" \"%i\")"

is not recursive, right?

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Hello,

Samuel Wales <samologist@gmail.com> writes:

> just to clarify,
>
>   "%(alpha-org-protocol-string \"%:link\" \"%:description\" \"%i\")"
>
> is not recursive, right?

It is. 

The first two are documented in org-capture-templates' docstring. So
I guess they are fine. However, the last parameter is problematic. As
pointed out by the OP, since it basically means "%(...)" are evaluated
after "%i", it can lead to the following chain of events:

  %i => %(evil-command) => Evil result.


Regards,

-- 
Nicolas Goaziou

Re: Org campture recursively expands %-escapes

[Samuel Wales]

On 11/26/15, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> Samuel Wales <samologist@gmail.com> writes:
>> just to clarify,
>>
>>   "%(alpha-org-protocol-string \"%:link\" \"%:description\" \"%i\")"
>>
>> is not recursive, right?
>
> It is.

the above is the only way i was able to get org-protocol to do what i needed.

> The first two are documented in org-capture-templates' docstring. So
> I guess they are fine. However, the last parameter is problematic. As
> pointed out by the OP, since it basically means "%(...)" are evaluated
> after "%i", it can lead to the following chain of events:
>
>   %i => %(evil-command) => Evil result.

i think i understand.  maybe we need another way to pass %i.

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Samuel Wales <samologist@gmail.com> writes:

> the above is the only way i was able to get org-protocol to do what
> i needed.

OK.

> i think i understand.  maybe we need another way to pass %i.

I guess we could restict "%()" evaluation to the Sexp defined in the
initial template.

Regards,

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

>> i think i understand.  maybe we need another way to pass %i.
>
> I guess we could restict "%()" evaluation to the Sexp defined in the
> initial template.

I committed a patch along those lines in master. Please let me know if
anything goes wrong with capture templates.

Re: Org campture recursively expands %-escapes

[Michael Brand]

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

Hi Nicolas

On Sun, Nov 29, 2015 at 5:00 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> I committed a patch along those lines in master. Please let me know if
> anything goes wrong with capture templates.

release_8.3.2-350-gbd3a2cb introduces a regression of "%()" in Org
feed templates. See
- commit release_7.8.10-1057-g042db37
- (commit release_7.8.10-1068-ge1d5a31)
- http://thread.gmane.org/gmane.emacs.orgmode/56991

I just wrote an ERT for this functionality, can you please add it?:

(should
 (string=
  "5 % Less (See\n Item \"3)\" Somewhere)\n Of THE [[http://orgmode.org]]\n
[2016-01-02 Sat]"
  (org-feed-format-entry
   '(:title "5 % less (see\n item \"3)\" somewhere)"  ; %h
     :link "http://ORGMODE.org"  ; %a
     :pubDate "Sat, 02 Jan 2016 12:00:00 +0000")  ; %u
   "%(capitalize \"%h\n of\") THE %(downcase \"%a\") %u"
   nil)))

Michael

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

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> release_8.3.2-350-gbd3a2cb introduces a regression of "%()" in Org
> feed templates. See
> - commit release_7.8.10-1057-g042db37
> - (commit release_7.8.10-1068-ge1d5a31)
> - http://thread.gmane.org/gmane.emacs.orgmode/56991
>
> I just wrote an ERT for this functionality, can you please add it?:
>
> (should
>  (string=
>   "5 % Less (See\n Item \"3)\" Somewhere)\n Of THE [[http://orgmode.org]]\n
> [2016-01-02 Sat]"
>   (org-feed-format-entry
>    '(:title "5 % less (see\n item \"3)\" somewhere)"  ; %h
>      :link "http://ORGMODE.org"  ; %a
>      :pubDate "Sat, 02 Jan 2016 12:00:00 +0000")  ; %u
>    "%(capitalize \"%h\n of\") THE %(downcase \"%a\") %u"
>    nil)))

I read the thread but I'm not sure where the regression is. Could you
elaborate on what is failing here?

Thank you.

-- 
Nicolas Goaziou

Re: Org campture recursively expands %-escapes

[Michael Brand]

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

Hi Nicolas

On Sun, Jan 3, 2016 at 8:51 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
>
> Michael Brand <michael.ch.brand@gmail.com> writes:
>
> > release_8.3.2-350-gbd3a2cb introduces a regression of "%()" in Org
> > feed templates. See
> > - commit release_7.8.10-1057-g042db37
> > - (commit release_7.8.10-1068-ge1d5a31)
> > - http://thread.gmane.org/gmane.emacs.orgmode/56991
> >
> > I just wrote an ERT for this functionality, can you please add it?:
> >
> > (should
> >  (string=
> >   "5 % Less (See\n Item \"3)\" Somewhere)\n Of THE [[http://orgmode.org
]]\n
> > [2016-01-02 Sat]"
> >   (org-feed-format-entry
> >    '(:title "5 % less (see\n item \"3)\" somewhere)"  ; %h
> >      :link "http://ORGMODE.org"  ; %a
> >      :pubDate "Sat, 02 Jan 2016 12:00:00 +0000")  ; %u
> >    "%(capitalize \"%h\n of\") THE %(downcase \"%a\") %u"
> >    nil)))
>
> I read the thread but I'm not sure where the regression is. Could you
> elaborate on what is failing here?

The above ERT that I would like to be added succeeds on
release_8.3.2-349-gbd79085 and fails on release_8.3.2-350-gbd3a2cb.

Michael

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

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Michael Brand <michael.ch.brand@gmail.com> writes:

> On Sun, Jan 3, 2016 at 8:51 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
> wrote:
>>
>> Michael Brand <michael.ch.brand@gmail.com> writes:
>>
>> > release_8.3.2-350-gbd3a2cb introduces a regression of "%()" in Org
>> > feed templates. See
>> > - commit release_7.8.10-1057-g042db37
>> > - (commit release_7.8.10-1068-ge1d5a31)
>> > - http://thread.gmane.org/gmane.emacs.orgmode/56991
>> >
>> > I just wrote an ERT for this functionality, can you please add it?:
>> >
>> > (should
>> >  (string=
>> >   "5 % Less (See\n Item \"3)\" Somewhere)\n Of THE [[http://orgmode.org
> ]]\n
>> > [2016-01-02 Sat]"
>> >   (org-feed-format-entry
>> >    '(:title "5 % less (see\n item \"3)\" somewhere)"  ; %h
>> >      :link "http://ORGMODE.org"  ; %a
>> >      :pubDate "Sat, 02 Jan 2016 12:00:00 +0000")  ; %u
>> >    "%(capitalize \"%h\n of\") THE %(downcase \"%a\") %u"
>> >    nil)))
>>
>> I read the thread but I'm not sure where the regression is. Could you
>> elaborate on what is failing here?
>
> The above ERT that I would like to be added succeeds on
> release_8.3.2-349-gbd79085 and fails on release_8.3.2-350-gbd3a2cb.

The only error I get is a difference of indentation before the date. Am
I missing something ?


Regards,

Re: Org campture recursively expands %-escapes

[Michael Brand]

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

Hi Nicolas

On Sun, Jan 3, 2016 at 10:07 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> The only error I get is a difference of indentation before the date. Am
> I missing something ?

When you evaluate

(progn
  (require 'org-feed)
  (org-feed-format-entry
   '(:title "5 % less (see\n item \"3)\" somewhere)"  ; %h
     :link "http://ORGMODE.org"  ; %a
     :pubDate "Sat, 02 Jan 2016 12:00:00 +0000")  ; %u
   "%(capitalize \"%h\n of\") THE %(downcase \"%a\") %u"
   nil))

do you get the expected "Lisp error: (void-function
org-capture-expand-embedded-elisp)" like me on
release_8.3.2-350-gbd3a2cb and today's release_8.3.2-441-ga87dea3 ?

If not I don't understand what is wrong with my git repo.

(After a rename I get

"%(capitalize \"5 % less (see
 item \\\"3)\\\" somewhere)
 of\") THE %(downcase \"[[http://ORGMODE.org]]
\") [2016-01-02 Sat]"

and on release_8.3.2-349-gbd79085 I get the expanded

"5 % Less (See
 Item \"3)\" Somewhere)
 Of THE [[http://orgmode.org]]
 [2016-01-02 Sat]"

)

Michael

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

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Michael Brand <michael.ch.brand@gmail.com> writes:

> When you evaluate
>
> (progn
>   (require 'org-feed)
>   (org-feed-format-entry
>    '(:title "5 % less (see\n item \"3)\" somewhere)"  ; %h
>      :link "http://ORGMODE.org"  ; %a
>      :pubDate "Sat, 02 Jan 2016 12:00:00 +0000")  ; %u
>    "%(capitalize \"%h\n of\") THE %(downcase \"%a\") %u"
>    nil))
>
> do you get the expected "Lisp error: (void-function
> org-capture-expand-embedded-elisp)" like me on
> release_8.3.2-350-gbd3a2cb and today's release_8.3.2-441-ga87dea3 ?

I do. Somehow, I didn't see this before. This is because I renamed
`org-capture-expand-embedded-elisp' into
`org-capture--expand-embedded-elisp'.

However, ISTM there is some code duplication between
`org-feed-format-entry' and `org-capture-fill-template'. Is calling
`org-capture-expand-embedded-elisp' from the former the right thing to
do?

Regards,

Re: Org campture recursively expands %-escapes

[Michael Brand]

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

Hi Nicolas

On Wed, Jan 6, 2016 at 12:39 AM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> I do. Somehow, I didn't see this before. This is because I renamed
> `org-capture-expand-embedded-elisp' into
> `org-capture--expand-embedded-elisp'.
>
> However, ISTM there is some code duplication between
> `org-feed-format-entry' and `org-capture-fill-template'. Is calling
> `org-capture-expand-embedded-elisp' from the former the right thing to
> do?

I don't really know. It was only my choice of how to avoid code
duplication between `org-feed-format-entry' and
`org-capture-fill-template' according to
http://thread.gmane.org/gmane.emacs.orgmode/56991/focus=57265
and with lack of better knowledge from my side. If there is a better
solution please go ahead.

Michael

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

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> I don't really know. It was only my choice of how to avoid code
> duplication between `org-feed-format-entry' and
> `org-capture-fill-template' according to
> http://thread.gmane.org/gmane.emacs.orgmode/56991/focus=57265
> and with lack of better knowledge from my side. If there is a better
> solution please go ahead.

For now, I left some code duplication. The issue is, hopefully, fixed,
however.

I didn't add your ert, but if you provide a patch with
a "test-org-feed.el" file, I can add it. It would be best to split the
test into small ones, though, as it seems you're testing multiple things
at once.

Regards,

-- 
Nicolas Goaziou

Re: Org campture recursively expands %-escapes

[Michael Brand]

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

Hi Nicolas

On Thu, Jan 7, 2016 at 5:35 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> For now, I left some code duplication. The issue is, hopefully, fixed,
> however.

It is, thank you!

> I didn't add your ert, but if you provide a patch with
> a "test-org-feed.el" file, I can add it. It would be best to split the
> test into small ones, though, as it seems you're testing multiple things
> at once.

I'm porting test-org-capture.el to test-org-feed.el and found this
issue in org-feed-format-entry: Evaluation of

    (org-feed-format-entry '(:title "success!") "\\\\%h" nil)

with Emacs 24.5 results in "Lisp error: (args-out-of-range #<buffer
*temp*> 4 5)". The Lisp error disappears when single stepping with
Edebug but then org-feed-format-entry returns "\\%h" instead of the
expected "\\success!". To my understanding the problem seems to be
that org-capture-escaped-% messes up the match data which leads to an
empty variable with the name "replacement". I wonder why this problem
is not showing up in org-capture-fill-template too.

Michael

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

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Michael Brand <michael.ch.brand@gmail.com> writes:

> I'm porting test-org-capture.el to test-org-feed.el and found this
> issue in org-feed-format-entry: Evaluation of
>
>     (org-feed-format-entry '(:title "success!") "\\\\%h" nil)
>
> with Emacs 24.5 results in "Lisp error: (args-out-of-range #<buffer
> *temp*> 4 5)". The Lisp error disappears when single stepping with
> Edebug but then org-feed-format-entry returns "\\%h" instead of the
> expected "\\success!". To my understanding the problem seems to be
> that org-capture-escaped-% messes up the match data which leads to an
> empty variable with the name "replacement". I wonder why this problem
> is not showing up in org-capture-fill-template too.

Fixed, too. Thank you.

Regards,

Re: Org campture recursively expands %-escapes

[Michael Brand]

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

Hi Nicolas

On Fri, Jan 8, 2016 at 11:44 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
>
> Michael Brand <michael.ch.brand@gmail.com> writes:
>
> > I'm porting test-org-capture.el to test-org-feed.el and found this
> > issue in org-feed-format-entry: Evaluation of
> >
> >     (org-feed-format-entry '(:title "success!") "\\\\%h" nil)
> >
> > with Emacs 24.5 results in "Lisp error: (args-out-of-range #<buffer
> > *temp*> 4 5)". The Lisp error disappears when single stepping with
> > Edebug but then org-feed-format-entry returns "\\%h" instead of the
> > expected "\\success!". To my understanding the problem seems to be
> > that org-capture-escaped-% messes up the match data which leads to an
> > empty variable with the name "replacement". I wonder why this problem
> > is not showing up in org-capture-fill-template too.
>
> Fixed, too. Thank you.

Yes, thank you.

On the other hand commit release_8.3.3-415-ge2fbaee breaks the ERT
that I suggested in this thread or its simplification here:

  (progn
    (require 'org-feed)
    (equal "\"A)" (org-feed-format-entry
                   '(:title "\"a)") "%(capitalize \"%h\")" nil)))

Michael

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

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> On the other hand commit release_8.3.3-415-ge2fbaee breaks the ERT
> that I suggested in this thread or its simplification here:
>
>   (progn
>     (require 'org-feed)
>     (equal "\"A)" (org-feed-format-entry
>                    '(:title "\"a)") "%(capitalize \"%h\")" nil)))

Hopefully, this is now fixed in master.

Regards,

-- 
Nicolas Goaziou

Re: Org campture recursively expands %-escapes

[Michael Brand]

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

Hi Nicolas

On Sat, Jan 9, 2016 at 5:05 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
>
> Michael Brand <michael.ch.brand@gmail.com> writes:
>
> > On the other hand commit release_8.3.3-415-ge2fbaee breaks the ERT
> > that I suggested in this thread or its simplification here:
> >
> >   (progn
> >     (require 'org-feed)
> >     (equal "\"A)" (org-feed-format-entry
> >                    '(:title "\"a)") "%(capitalize \"%h\")" nil)))
>
> Hopefully, this is now fixed in master.

It is, thank you. Incredibly fast 11 minutes from my report to your
commit!

My current ERT for test-org-feed.el

   (equal
    "5 % Less (See\n Item \"3)\" Somewhere)"
    (org-feed-format-entry
     '(:title "5 % less (see\n item \"3)\" somewhere)")
     "%(capitalize \"%h\")" nil))

works now too. What does not work yet is my backport of the above ERT
to test-org-capture.el:

   (equal
    "5 % Less (See\n Item \"3)\" Somewhere)\n"
    (let ((org-store-link-plist nil))
      (org-capture-fill-template
       "%(capitalize \"%i\")"
       "5 % less (see\n item \"3)\" somewhere)")))

Am I doing something wrong?

Michael

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

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Michael Brand <michael.ch.brand@gmail.com> writes:

> My current ERT for test-org-feed.el
>
>    (equal
>     "5 % Less (See\n Item \"3)\" Somewhere)"
>     (org-feed-format-entry
>      '(:title "5 % less (see\n item \"3)\" somewhere)")
>      "%(capitalize \"%h\")" nil))
>
> works now too. What does not work yet is my backport of the above ERT
> to test-org-capture.el:
>
>    (equal
>     "5 % Less (See\n Item \"3)\" Somewhere)\n"
>     (let ((org-store-link-plist nil))
>       (org-capture-fill-template
>        "%(capitalize \"%i\")"
>        "5 % less (see\n item \"3)\" somewhere)")))
>
> Am I doing something wrong?

I think you're mis-using "%i" place-holder. One feature is to repeat the
leading text, so that, when you write, for example "> %i", "> " is
repeated every line.

Regards,

Re: Org campture recursively expands %-escapes

[Michael Brand]

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

Hi Nicolas

On Sat, Jan 9, 2016 at 6:54 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
>
> Michael Brand <michael.ch.brand@gmail.com> writes:
>
> > My current ERT for test-org-feed.el
> >
> >    (equal
> >     "5 % Less (See\n Item \"3)\" Somewhere)"
> >     (org-feed-format-entry
> >      '(:title "5 % less (see\n item \"3)\" somewhere)")
> >      "%(capitalize \"%h\")" nil))
> >
> > works now too. What does not work yet is my backport of the above ERT
> > to test-org-capture.el:
> >
> >    (equal
> >     "5 % Less (See\n Item \"3)\" Somewhere)\n"
> >     (let ((org-store-link-plist nil))
> >       (org-capture-fill-template
> >        "%(capitalize \"%i\")"
> >        "5 % less (see\n item \"3)\" somewhere)")))
> >
> > Am I doing something wrong?
>
> I think you're mis-using "%i" place-holder. One feature is to repeat the
> leading text, so that, when you write, for example "> %i", "> " is
> repeated every line.

Indeed. After removing the "\n" from the input of the capture ERT it
works too.

I would like to push the attached change to add some ERTs with the
commit msg below and would like to ask you for a review first.

Michael

----------------------------------------

Add ERTs for feed templates

* testing/lisp/test-org-capture.el (test-org-capture/fill-template):
  Strengthen some expectations, add new tests.

* testing/lisp/test-org-feed.el: New file derived from
  testing/lisp/test-org-capture.el.

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

[-- Attachment #2: git.diff --]
[-- Type: text/plain, Size: 6535 bytes --]

diff --git a/testing/lisp/test-org-capture.el b/testing/lisp/test-org-capture.el
index 714309d..e1011d0 100644
--- a/testing/lisp/test-org-capture.el
+++ b/testing/lisp/test-org-capture.el
@@ -26,7 +26,9 @@
 (require 'org-capture)
 
 (ert-deftest test-org-capture/fill-template ()
-  "Test `org-capture-fill-template' specifications."
+  "Test `org-capture-fill-template' specifications.
+The tests here are very similar to those in
+`test-org-feed/fill-template'."
   ;; %(sexp) placeholder.
   (should
    (equal "success!\n"
@@ -44,12 +46,14 @@
 	  (org-capture-fill-template "%T")))
   ;; %u and %U placeholders.
   (should
-   (string-match-p
-    (format-time-string (substring (car org-time-stamp-formats) 1 -1))
+   (equal
+    (concat "[" (format-time-string
+		 (substring (car org-time-stamp-formats) 1 -1)) "]\n")
     (org-capture-fill-template "%u")))
   (should
-   (string-match-p
-    (format-time-string (substring (cdr org-time-stamp-formats) 1 -1))
+   (equal
+    (concat "[" (format-time-string
+		 (substring (cdr org-time-stamp-formats) 1 -1)) "]\n")
     (org-capture-fill-template "%U")))
   ;; %i placeholder.  Make sure sexp placeholders are not expanded
   ;; when they are inserted through this one.
@@ -57,11 +61,12 @@
    (equal "success!\n"
 	  (let ((org-store-link-plist nil))
 	    (org-capture-fill-template "%i" "success!"))))
-  (should-not
-   (equal "failure!\n"
+  (should
+   (equal "%(concat \"no \" \"evaluation\")\n"
 	  (let ((org-store-link-plist nil))
-	    (org-capture-fill-template "%i" "%(concat \"failure\" \"!\")"))))
-  ;; Test %-escaping with / character.
+	    (org-capture-fill-template
+	     "%i" "%(concat \"no \" \"evaluation\")"))))
+  ;; Test %-escaping with \ character.
   (should
    (equal "%i\n"
 	  (let ((org-store-link-plist nil))
@@ -73,7 +78,21 @@
   (should
    (equal "\\%i\n"
 	  (let ((org-store-link-plist nil))
-	    (org-capture-fill-template "\\\\\\%i" "success!")))))
+	    (org-capture-fill-template "\\\\\\%i" "success!"))))
+  ;; More than one placeholder in the same template.
+  (should
+   (equal "success! success! success! success!\n"
+	  (let ((org-store-link-plist nil))
+	    (org-capture-fill-template "%i %i %i %i" "success!"))))
+  ;; %(sexp) placeholder with an input containing the traps %, " and )
+  ;; all at once which is complicated to parse.
+  (should
+   (equal
+    "5 % Less (See Item \"3)\" Somewhere)\n"
+    (let ((org-store-link-plist nil))
+      (org-capture-fill-template
+       "%(capitalize \"%i\")"
+       "5 % less (see item \"3)\" somewhere)")))))
 
 
 
diff --git a/testing/lisp/test-org-feed.el b/testing/lisp/test-org-feed.el
index e69de29..6696f95 100644
--- a/testing/lisp/test-org-feed.el
+++ b/testing/lisp/test-org-feed.el
@@ -0,0 +1,110 @@
+;;; test-org-feed.el --- Tests for org-feed.el -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016  Michael Brand
+
+;; Author: Michael Brand <michael.ch.brand@gmail.com>
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Unit tests for Org Feed library.
+
+;;; Code:
+
+(require 'org-feed)
+
+(ert-deftest test-org-feed/fill-template ()
+  "Test `org-feed-format-entry' template specifications.
+The tests here are very similar to those in
+`test-org-capture/fill-template'."
+  ;; %(sexp) placeholder.
+  (should
+   (equal "success!"
+	  (org-feed-format-entry nil "%(concat \"success\" \"!\")" nil)))
+  ;; %a placeholder.
+  (should
+   (equal "[[http://orgmode.org]]\n"
+	  (org-feed-format-entry '(:link "http://orgmode.org") "%a" nil)))
+  ;; %t and %T placeholders.
+  (should
+   (equal (format-time-string (car org-time-stamp-formats))
+	  (org-feed-format-entry nil "%t" nil)))
+  (should
+   (equal "<2016-01-02 Sat>"
+	  (org-feed-format-entry
+	   '(:pubDate "Sat, 02 Jan 2016 12:00:00 +0000") "%t" nil)))
+  (should
+   (equal (format-time-string (cdr org-time-stamp-formats))
+	  (org-feed-format-entry nil "%T" nil)))
+  (should
+   (equal "<2016-01-02 Sat 12:00>"
+	  (org-feed-format-entry
+	   '(:pubDate "Sat, 02 Jan 2016 12:00:00 +0000") "%T" nil)))
+  ;; %u and %U placeholders.
+  (should
+   (equal
+    (concat "[" (format-time-string
+		 (substring (car org-time-stamp-formats) 1 -1)) "]")
+    (org-feed-format-entry nil "%u" nil)))
+  (should
+   (equal "[2016-01-02 Sat]"
+	  (org-feed-format-entry
+	   '(:pubDate "Sat, 02 Jan 2016 12:00:00 +0000") "%u" nil)))
+  (should
+   (equal
+    (concat "[" (format-time-string
+		 (substring (cdr org-time-stamp-formats) 1 -1)) "]")
+    (org-feed-format-entry nil "%U" nil)))
+  (should
+   (equal "[2016-01-02 Sat 12:00]"
+	  (org-feed-format-entry
+	   '(:pubDate "Sat, 02 Jan 2016 12:00:00 +0000") "%U" nil)))
+  ;; %h placeholder.  Make sure sexp placeholders are not expanded
+  ;; when they are inserted through this one.
+  (should
+   (equal "success!"
+	  (org-feed-format-entry '(:title "success!") "%h" nil)))
+  (should
+   (equal "%(concat \"no \" \"evaluation\")"
+	  (org-feed-format-entry
+	   '(:title "%(concat \"no \" \"evaluation\")") "%h" nil)))
+  ;; Test %-escaping with \ character.
+  (should
+   (equal "%h"
+	  (org-feed-format-entry '(:title "success!") "\\%h" nil)))
+  (should
+   (equal "\\success!"
+	  (org-feed-format-entry '(:title "success!") "\\\\%h" nil)))
+  (should
+   (equal "\\%h"
+	  (org-feed-format-entry '(:title "success!") "\\\\\\%h" nil)))
+  ;; More than one placeholder in the same template.
+  (should
+   (equal "success! success! success! success!"
+	  (org-feed-format-entry '(:title "success!") "%h %h %h %h" nil)))
+  ;; %(sexp) placeholder with an input containing the traps %, ", )
+  ;; and \n all at once which is complicated to parse.
+  (should
+   (equal
+    "5 % Less (See\n Item \"3)\" Somewhere)"
+    (org-feed-format-entry
+     '(:title "5 % less (see\n item \"3)\" somewhere)")
+     "%(capitalize \"%h\")" nil))))
+
+
+
+
+(provide 'test-org-feed)
+;;; test-org-feed.el ends here

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> I would like to push the attached change to add some ERTs with the
> commit msg below and would like to ask you for a review first.

It looks good. Thank you. Minor suggestions follow.

>  (ert-deftest test-org-capture/fill-template ()
> -  "Test `org-capture-fill-template' specifications."
> +  "Test `org-capture-fill-template' specifications.
> +The tests here are very similar to those in
> +`test-org-feed/fill-template'."

Not sure the last sentence above is really interesting. Ditto for the
other occurrences.

> -   (string-match-p
> -    (format-time-string (substring (car org-time-stamp-formats) 1 -1))
> +   (equal
> +    (concat "[" (format-time-string
> +		 (substring (car org-time-stamp-formats) 1 -1)) "]\n")
>      (org-capture-fill-template "%u")))
>    (should
> -   (string-match-p
> -    (format-time-string (substring (cdr org-time-stamp-formats) 1 -1))
> +   (equal
> +    (concat "[" (format-time-string
> +		 (substring (cdr org-time-stamp-formats) 1 -1)) "]\n")

I discovered recently (!) `org-time-stamp-formats' which avoids doing
the substring dance. You may want to use it instead. Ditto for the other
occurrences.


Regards,

-- 
Nicolas Goaziou

Re: Org campture recursively expands %-escapes

[Michael Brand]

Hi Nicolas

On Tue, Jan 12, 2016 at 12:05 AM, Nicolas Goaziou
<mail@nicolasgoaziou.fr> wrote:
>
> Michael Brand <michael.ch.brand@gmail.com> writes:
>
>>  (ert-deftest test-org-capture/fill-template ()
>> -  "Test `org-capture-fill-template' specifications."
>> +  "Test `org-capture-fill-template' specifications.
>> +The tests here are very similar to those in
>> +`test-org-feed/fill-template'."
>
> Not sure the last sentence above is really interesting. Ditto for the
> other occurrences.

Maybe this and vice versa is better?:

    (ert-deftest test-org-capture/fill-template ()
      "Test `org-capture-fill-template' specifications."

      ;; When working on these tests consider to also change
      ;; `test-org-feed/fill-template'.

      ;; %(sexp) placeholder.
      (should
      [...]

>> -   (string-match-p
>> -    (format-time-string (substring (car org-time-stamp-formats) 1 -1))
>> +   (equal
>> +    (concat "[" (format-time-string
>> +              (substring (car org-time-stamp-formats) 1 -1)) "]\n")
>>      (org-capture-fill-template "%u")))
>>    (should
>> -   (string-match-p
>> -    (format-time-string (substring (cdr org-time-stamp-formats) 1 -1))
>> +   (equal
>> +    (concat "[" (format-time-string
>> +              (substring (cdr org-time-stamp-formats) 1 -1)) "]\n")
>
> I discovered recently (!) `org-time-stamp-formats' which avoids doing
> the substring dance. You may want to use it instead. Ditto for the other
> occurrences.

I don't understand because the org-time-stamp-formats you mention is
already used and does not cover inactive timestamps.

Michael

Re: Org campture recursively expands %-escapes

[Nicolas Goaziou]

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> Maybe this and vice versa is better?:
>
>     (ert-deftest test-org-capture/fill-template ()
>       "Test `org-capture-fill-template' specifications."
>
>       ;; When working on these tests consider to also change
>       ;; `test-org-feed/fill-template'.
>
>       ;; %(sexp) placeholder.
>       (should
>       [...]

Clearer, IMO.

> I don't understand because the org-time-stamp-formats you mention is
> already used and does not cover inactive timestamps.

I'm talking about the function, not the variable.

Regards,

-- 
Nicolas Goaziou

Re: Org campture recursively expands %-escapes

[Michael Brand]

Hi Nicolas

On Tue, Jan 12, 2016 at 9:42 AM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
>
> Michael Brand <michael.ch.brand@gmail.com> writes:
>
> > I don't understand because the org-time-stamp-formats you mention is
> > already used and does not cover inactive timestamps.
>
> I'm talking about the function, not the variable.

Now I found it: The name of the function you mention is not
`org-time-stamp-formats' but
`org-time-stamp-format' without the "s" at the end.

I pushed my change, thank you for your help.

Michael