Michael Brand | 16 Jun 2012 17:53
Picon

Add the capture feature "%(sexp)" to org-feed

Hi all

I am almost done with adding the capture feature "%(sexp)" to
org-feed.el. Because for this I would like to use
org-capture-escaped-% from org-capture.el in org-feed.el I renamed
this function to org-template-escaped-% and call it in org-feed.el.
The only thing what is missing is that unless I do some manual
evaluations before, there is still "Symbol's function definition is
void: org-template-escaped-%". For this I would like to ask for some
design and implementation guidance, if available also where I should
read in the elisp reference manual or wherever.

Michael

Nick Dokos | 17 Jun 2012 04:07
Picon
Favicon

Re: Add the capture feature "%(sexp)" to org-feed

Michael Brand <michael.ch.brand <at> gmail.com> wrote:

> I am almost done with adding the capture feature "%(sexp)" to
> org-feed.el. Because for this I would like to use
> org-capture-escaped-% from org-capture.el in org-feed.el I renamed
> this function to org-template-escaped-% and call it in org-feed.el.
> The only thing what is missing is that unless I do some manual
> evaluations before, there is still "Symbol's function definition is
> void: org-template-escaped-%". For this I would like to ask for some
> design and implementation guidance, if available also where I should
> read in the elisp reference manual or wherever.
> 

Either require org-capture in org-feed or move the function to org.el, which
both org-capture and org-feed require. 

Nick

Michael Brand | 24 Jun 2012 19:51
Picon

Re: Add the capture feature "%(sexp)" to org-feed

Hi all

On Sun, Jun 17, 2012 at 4:07 AM, Nick Dokos <nicholas.dokos <at> hp.com> wrote:
> Either require org-capture in org-feed or move the function to org.el, which
> both org-capture and org-feed require.

Thank you for the help. I have chosen the solution with require org-capture.

The patch is now finished and attached:

Add the capture feature "%(sexp)" to org-feed

* lisp/org-capture.el (org-capture-fill-template): Fold the code for
embedded elisp $(sexp) into the following new function and constant.
(org-capture-template-embedded-elisp-re): New constant, contains the
already existing common regexp.
(org-capture-eval-and-replace-embedded-elisp): New function, contains the
already existing common code.
(org-capture-inside-embedded-elisp-p): New function to tell whether the
point is inside embedded elisp %(sexp).
* lisp/org-feed.el (org-feed-default-template): Add the new functionality
to the docstring.
(org-feed-format-entry): Add require org-capture to access the new common
function.  Add escaping of `"' to "simple %-escapes" if necessary for
embedded lisp %(sexp).  Add evaluation and replacement of embedded lisp
%(sexp).

Support for example %(capitalize \\\"%h\\\")" in the template of org-feed.

Michael
(Continue reading)

Bastien | 8 Aug 2012 01:05
Picon
Gravatar

Re: Add the capture feature "%(sexp)" to org-feed

Hi Michael,

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

> Thank you for the help. I have chosen the solution with require org-capture.
>
> The patch is now finished and attached:

I've reworked some parts of your patch, specifically, I used
`org-at-regexp-p' instead of `org-capture-inside-embedded-elisp-p'.

Can you check this is working okay for you?

Thanks!


--

-- 
 Bastien
Michael Brand | 9 Aug 2012 16:07
Picon

Re: Add the capture feature "%(sexp)" to org-feed

Hi Bastien

Thank you for reviewing my patch.

On Wed, Aug 8, 2012 at 1:05 AM, Bastien <bzg <at> gnu.org> wrote:

> I've reworked some parts of your patch,

You omitted the Local variables sentence-end-double-space: t. Isn't
this a good idea for all the users like me that have set this to nil
in their config? I wanted to add it to more files as soon as I will
change them.

“The current date.” is probably meant to remain “the current date.”.

> specifically, I used `org-at-regexp-p' instead of
> `org-capture-inside-embedded-elisp-p'.

With `org-capture-inside-embedded-elisp-p' I wanted to be quite more
waterproof than `org-at-regexp-p' with the new additional closing
parenthesis in `org-capture-template-embedded-elisp-re'. See its
comment “to deal with for example %(length ")")”. Isn't it a good idea
to use `forward-sexp' in Emacs Lisp Mode for this? What can I improve?

> Can you check this is working okay for you?

I'll do with the final version of the patch if there will be changes.

Michael

(Continue reading)

Bastien | 9 Aug 2012 16:53
Picon
Gravatar

Re: Add the capture feature "%(sexp)" to org-feed

Hi Michael,

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

> You omitted the Local variables sentence-end-double-space: t. Isn't
> this a good idea for all the users like me that have set this to nil
> in their config? I wanted to add it to more files as soon as I will
> change them.

I've check against current Emacs trunk, and only three files/packages
uses this: rst.el, filesets.el and mh-e.

So I'd rather not pollute the hundred files of Org with this.

> “The current date.” is probably meant to remain “the current date.”.

Yes.

>> specifically, I used `org-at-regexp-p' instead of
>> `org-capture-inside-embedded-elisp-p'.
>
> With `org-capture-inside-embedded-elisp-p' I wanted to be quite more
> waterproof than `org-at-regexp-p' with the new additional closing
> parenthesis in `org-capture-template-embedded-elisp-re'. See its
> comment “to deal with for example %(length ")")”. Isn't it a good idea
> to use `forward-sexp' in Emacs Lisp Mode for this? What can I improve?

(org-at-regexp-p org-capture-template-embedded-elisp-re) already 
returns the correct value for %(length ")") -- but you're right there 
is a problem with my patch: `org-at-regexp-p' does not match over 
(Continue reading)

Michael Brand | 9 Aug 2012 20:00
Picon

Re: Add the capture feature "%(sexp)" to org-feed

Hi Bastien

On Thu, Aug 9, 2012 at 4:53 PM, Bastien <bzg <at> gnu.org> wrote:

> I've check against current Emacs trunk, and only three files/packages
> uses this: rst.el, filesets.el and mh-e.
>
> So I'd rather not pollute the hundred files of Org with this.

That's all right.

> (org-at-regexp-p org-capture-template-embedded-elisp-re) already
> returns the correct value for %(length ")")

For the above example yes, but not with more than one %():
“- %(capitalize "simple percent-escape")\n- %(capitalize "one more")”

Also non-greedy regexp is not enough here:
“%(capitalize "(some) text")”

> -- but you're right there
> is a problem with my patch: `org-at-regexp-p' does not match over
> multiple lines.  Maybe you can play with `org-in-regexp':
>
>   (org-in-regexp org-capture-template-embedded-elisp-re 3)
>
> If using `forward-sexp' is necessary let's do so -- but I thought it
> was too complex first.

When I wrote the patch my conclusion was that regexp alone is not
(Continue reading)

Bastien | 10 Aug 2012 10:53
Picon
Gravatar

Re: Add the capture feature "%(sexp)" to org-feed

Hi Michael,

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

> Maybe there are still some simplifications?

Please try the attached patch and let me know if it works.

If you're okay, I'll then apply it under your name.


--

-- 
 Bastien
Michael Brand | 11 Aug 2012 17:31
Picon

Re: Add the capture feature "%(sexp)" to org-feed

Hi Bastien

On Fri, Aug 10, 2012 at 10:53 AM, Bastien <bzg <at> gnu.org> wrote:
> Please try the attached patch and let me know if it works.

Thank you for making `org-capture-inside-embedded-elisp-p' much
clearer. I was not sure if you really wanted to omit switching to
Emacs Lisp Mode although it is useful for example for “%(length ")")”.
So I attach a new patch where I restored this part, now cleaner too.
Also a small change against your latest patch to not stop at “%”
without following “(” and possibly miss a “%(” before.

I temporarily emptied “:FEEDSTATUS:” in all my feeds and updated them
and also tried some odd artificial examples. All works the same as
with my first patch. Can you please review this new patch compared
against your latest and apply when appropriate?

Michael
From 142625bad45ca07a185d95caa0d810225e389b95 Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand <at> gmail.com>
Date: Sat, 11 Aug 2012 17:26:57 +0200
Subject: [PATCH] Add the capture feature sexp to org feed

* org-feed.el (org-feed-format-entry): Require `org-capture'.
Expand Elisp %(...) templates.
(org-feed-default-template): Update docstring.

* org-capture.el (org-capture-expand-embedded-elisp): New
(Continue reading)

Bastien | 11 Aug 2012 18:55
Picon
Gravatar

Re: Add the capture feature "%(sexp)" to org-feed

Hi Michael,

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

> Thank you for making `org-capture-inside-embedded-elisp-p' much
> clearer. I was not sure if you really wanted to omit switching to
> Emacs Lisp Mode although it is useful for example for “%(length ")")”.
> So I attach a new patch where I restored this part, now cleaner too.
> Also a small change against your latest patch to not stop at “%”
> without following “(” and possibly miss a “%(” before.

Thanks.  Actually we don't need the whole emacs-lisp-mode so that
forward-sexp do something sensible.  This is enough:

  (with-syntax-table emacs-lisp-mode-syntax-table ...)

See the commit I just pushed:

  http://orgmode.org/w/?p=org-mode.git;a=commit;h=042db3

> I temporarily emptied “:FEEDSTATUS:” in all my feeds and updated them
> and also tried some odd artificial examples. All works the same as
> with my first patch. Can you please review this new patch compared
> against your latest and apply when appropriate?

Done.  Thanks again for your patience,

--

-- 
 Bastien

(Continue reading)

Michael Brand | 11 Aug 2012 20:11
Picon

Re: Add the capture feature "%(sexp)" to org-feed

Hi Bastien

On Sat, Aug 11, 2012 at 6:55 PM, Bastien <bzg <at> gnu.org> wrote:
> Michael Brand <michael.ch.brand <at> gmail.com> writes:
>> [...]
>> Also a small change against your latest patch to not stop at “%”
>> without following “(” and possibly miss a “%(” before.

This part that I changed between
http://patchwork.newartisans.com/patch/1405
and
http://patchwork.newartisans.com/patch/1408
got lost, the attached patch restores it.

> Actually we don't need the whole emacs-lisp-mode so that
> forward-sexp do something sensible.  This is enough:
>
>   (with-syntax-table emacs-lisp-mode-syntax-table ...)

Thank you for the improvement, this is a very interesting `with-'.

Michael
From 10888cbd40e3f546b9e577ff59bfa152679f9628 Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand <at> gmail.com>
Date: Sat, 11 Aug 2012 20:08:35 +0200
Subject: [PATCH] Improve parsing of org-capture-inside-embedded-elisp-p

* org-capture.el (org-capture-inside-embedded-elisp-p): Improve parsing.
(Continue reading)

Bastien | 11 Aug 2012 23:59
Picon
Gravatar

Re: Add the capture feature "%(sexp)" to org-feed

Hi Michael,

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

> This part that I changed between
> http://patchwork.newartisans.com/patch/1405
> and
> http://patchwork.newartisans.com/patch/1408
> got lost, the attached patch restores it.

Unless I miss something obvious, the current version returns exactly 
the same thing than the one you propose -- can you double-check or let 
me know what is wrong with the current solution?

Thanks!

--

-- 
 Bastien

Michael Brand | 12 Aug 2012 11:37
Picon

Re: Add the capture feature "%(sexp)" to org-feed

Hi Bastien

On Sat, Aug 11, 2012 at 11:59 PM, Bastien <bzg <at> gnu.org> wrote:
> Unless I miss something obvious, the current version returns exactly
> the same thing than the one you propose -- can you double-check or let
> me know what is wrong with the current solution?

Your solution is wrong when there is a “%” not immediately followed by “(”:

On Sat, Aug 11, 2012 at 8:11 PM, Michael Brand
<michael.ch.brand <at> gmail.com> wrote:
> On Sat, Aug 11, 2012 at 6:55 PM, Bastien <bzg <at> gnu.org> wrote:
>> Michael Brand <michael.ch.brand <at> gmail.com> writes:
>>> [...]
>>> Also a small change against your latest patch to not stop at “%”
>>> without following “(” and possibly miss a “%(” before.

A realistic example, with point somewhere between “%” and the last “)”:
“- %(capitalize "5 % less (see item \"3)\" below)\n")\n”

This can happen with a feed template containing
"- %(capitalize \"%description\")\n"
and %description expanding to
“5 % less (see item "3)" below)\n”

All problems of the example are solved:
- escaping of the quotes that surround the “3)” in string
- seemingly unbalanced parenthesis in string coming from “3)”
- newline within %(...)
- “%” not immediately followed by “(”
(Continue reading)

Bastien | 12 Aug 2012 12:12
Gravatar

Re: Add the capture feature "%(sexp)" to org-feed

Hi Michael,

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

> Your solution is wrong when there is a “%” not immediately followed by
> “(”:

Argh, right.  I applied your patch, thanks again for the detailed
examples. 

--

-- 
 Bastien

Ivan Andrus | 9 Aug 2012 23:09
Picon
Gravatar

Re: Add the capture feature "%(sexp)" to org-feed

On Aug 9, 2012, at 4:53 PM, Bastien wrote:

> Hi Michael,
> 
> Michael Brand <michael.ch.brand <at> gmail.com> writes:
> 
>> You omitted the Local variables sentence-end-double-space: t. Isn't
>> this a good idea for all the users like me that have set this to nil
>> in their config? I wanted to add it to more files as soon as I will
>> change them.
> 
> I've check against current Emacs trunk, and only three files/packages
> uses this: rst.el, filesets.el and mh-e.
> 
> So I'd rather not pollute the hundred files of Org with this.

I'm probably missing something, but isn't this what directory local variables are for?

-Ivan

Michael Brand | 10 Aug 2012 06:48
Picon

Re: Add the capture feature "%(sexp)" to org-feed

Hi Ivan

On Thu, Aug 9, 2012 at 11:09 PM, Ivan Andrus <darthandrus <at> gmail.com> wrote:
> I'm probably missing something, but isn't this what directory local variables are for?

Yes. This is useful in general for me, thank you for the hint. I
propose the attached patch for the already existing .dir-locals.el and
.dir-settings.el.

Michael
From 345419858e85d75a96f73b7d7f6e0ddbde0aae81 Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand <at> gmail.com>
Date: Fri, 10 Aug 2012 06:40:10 +0200
Subject: [PATCH] Directory local variables: Set sentence-end-double-space to t

* .dir-locals.el: Add comments and set `sentence-end-double-space' to t.

* .dir-settings.el: Add comments and set `sentence-end-double-space' to t.
---
 .dir-locals.el   |    9 +++++++--
 .dir-settings.el |    9 +++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 7094cb9..05d50f6 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
 <at>  <at>  -1,4 +1,9  <at>  <at> 
(Continue reading)

Bastien | 10 Aug 2012 09:24
Picon
Gravatar

Re: Add the capture feature "%(sexp)" to org-feed

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

> Yes. This is useful in general for me, thank you for the hint. I
> propose the attached patch for the already existing .dir-locals.el and
> .dir-settings.el.

Applied, thanks to you and Ivan.

--

-- 
 Bastien


Gmane