[BUG] in org-property-drawer-re?

[BUG] in org-property-drawer-re?

[Thorsten Jolitz]

Hi List, 

for the navi-mode keyword-search for complete property drawers I copied

,-----------------------
| org-property-drawer-re
`-----------------------

from org.el:

#+begin_src emacs-lisp
  (concat "\\(" org-property-start-re "\\)[^\000]*\\("
	  org-property-end-re "\\)\n?")
#+end_src

#+results:
: \(^[ 	]*:PROPERTIES:[ 	]*$\)[^\\000]*\(^[ 	]*:END:[ 	]*$\)
: ?

A bit unreadable, but you get the message ... here is my hopefully equivalent
version: 

,--------------------------------------------------------------
| (:propertydrawer
|   . (concat "\\(^[\\s\\t]*:PROPERTIES:[\\s\\t]*$\\)[^\\000]*"
|             "\\(^[\\s\\t]*:END:[\\s\\t]*$\\)\\n?"))
`--------------------------------------------------------------

But this did not match correctly in Bernt Hansens tutorial:

    ,--------------------------------------------------------------------------
    |  43::PROPERTIES:
    |    ::CUSTOM_ID: Setup
    |    ::END:
    | 131::PROPERTIES:
    |    ::CUSTOM_ID: OrgFiles
    |    ::END:
    | 185::PROPERTIES:
    |    ::CUSTOM_ID: AgendaSetup
    |    ::END:
    |    :
    |    :Here is my current =org-agenda-files= setup.
    |    :#+begin_src emacs-lisp :tangle no
    |    :  (setq org-agenda-files (quote ("~/git/org"
    |    :                                 "~/git/org/client1"
    |    :                                 "~/git/org/bzflag"
    |    :                                 "~/git/client2")))
    |    :#+end_src
    |    :
    |    :#+begin_src emacs-lisp :tangle yes :exports none
    |    :  ;; The following setting is different from the document so that you
    |    :  ;; can override the document org-agenda-files by setting your
    |    :  ;; org-agenda-files in the variable org-user-agenda-files
    |    :  ;;
    |    :  (if (boundp 'org-user-agenda-files)
    |    :      (setq org-agenda-files org-user-agenda-files)
    |    :    (setq org-agenda-files (quote ("~/git/org"
    |    :                                 "~/git/org/client1"
    |    :                                 "~/git/org/bzflag"
    |    :                                 "~/git/client2"))))
    |    :  
    |    :#+end_src
    `--------------------------------------------------------------------------

I had to add two ? after the * and delete the final \n 

,-------------------------------------------------------------
| (:propertydrawer
| . (concat "\\(^[\\s\\t]*:PROPERTIES:[\\s\\t]*$\\)[^\\000]*?"
|           "\\(^[\\s\\t]*:END:[\\s\\t]*?$\\)"))
`-------------------------------------------------------------

to get the desired results:

    ,---------------------------------
    |  43::PROPERTIES:
    |    ::CUSTOM_ID: Setup
    |    ::END:
    | 131::PROPERTIES:
    |    ::CUSTOM_ID: OrgFiles
    |    ::END:
    | 185::PROPERTIES:
    |    ::CUSTOM_ID: AgendaSetup
    |    ::END:
    | 234::PROPERTIES:
    |    ::CUSTOM_ID: OrgFileStructure
    |    ::END:
    `---------------------------------

A bug in the regexp?

PS 
Can anybody explain this marvelous construct in the regexp:

,---------
| [^\\000]
`---------

I often pondered about how to achieve its effect with other means, since
I did not find it in the Emacs Lisp manual.

-- 
cheers,
Thorsten

Re: [BUG] in org-property-drawer-re?

[Carsten Dominik]

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


On 1.10.2013, at 19:50, Thorsten Jolitz <tjolitz@gmail.com> wrote:

> 
> Hi List, 
> 
> for the navi-mode keyword-search for complete property drawers I copied
> 
> ,-----------------------
> | org-property-drawer-re
> `-----------------------
> 
> from org.el:
> 
> #+begin_src emacs-lisp
>  (concat "\\(" org-property-start-re "\\)[^\000]*\\("
> 	  org-property-end-re "\\)\n?")
> #+end_src
> 
> #+results:
> : \(^[ 	]*:PROPERTIES:[ 	]*$\)[^\\000]*\(^[ 	]*:END:[ 	]*$\)
> : ?
> 
> A bit unreadable, but you get the message ... here is my hopefully equivalent
> version: 
> 
> ,--------------------------------------------------------------
> | (:propertydrawer
> |   . (concat "\\(^[\\s\\t]*:PROPERTIES:[\\s\\t]*$\\)[^\\000]*"
> |             "\\(^[\\s\\t]*:END:[\\s\\t]*$\\)\\n?"))
> `--------------------------------------------------------------
> 
> But this did not match correctly in Bernt Hansens tutorial:

Indeed, this is a bad regular expression, it is too greedy and will
match all the way to the last :END: line it can find.  also, \\s is
wrong, it should be just a space, so "[ \t]". Luckily
this regular expression does not seem to be used in Org as far
as I can see....

> 
>    ,--------------------------------------------------------------------------
>    |  43::PROPERTIES:
>    |    ::CUSTOM_ID: Setup
>    |    ::END:
>    | 131::PROPERTIES:
>    |    ::CUSTOM_ID: OrgFiles
>    |    ::END:
>    | 185::PROPERTIES:
>    |    ::CUSTOM_ID: AgendaSetup
>    |    ::END:
>    |    :
>    |    :Here is my current =org-agenda-files= setup.
>    |    :#+begin_src emacs-lisp :tangle no
>    |    :  (setq org-agenda-files (quote ("~/git/org"
>    |    :                                 "~/git/org/client1"
>    |    :                                 "~/git/org/bzflag"
>    |    :                                 "~/git/client2")))
>    |    :#+end_src
>    |    :
>    |    :#+begin_src emacs-lisp :tangle yes :exports none
>    |    :  ;; The following setting is different from the document so that you
>    |    :  ;; can override the document org-agenda-files by setting your
>    |    :  ;; org-agenda-files in the variable org-user-agenda-files
>    |    :  ;;
>    |    :  (if (boundp 'org-user-agenda-files)
>    |    :      (setq org-agenda-files org-user-agenda-files)
>    |    :    (setq org-agenda-files (quote ("~/git/org"
>    |    :                                 "~/git/org/client1"
>    |    :                                 "~/git/org/bzflag"
>    |    :                                 "~/git/client2"))))
>    |    :  
>    |    :#+end_src
>    `--------------------------------------------------------------------------
> 
> I had to add two ?

You only need the first - the second is OK.

> after the * and delete the final \n 
> 
> ,-------------------------------------------------------------
> | (:propertydrawer
> | . (concat "\\(^[\\s\\t]*:PROPERTIES:[\\s\\t]*$\\)[^\\000]*?"
> |           "\\(^[\\s\\t]*:END:[\\s\\t]*?$\\)"))
> `-------------------------------------------------------------

Yes, you need the star to make it non-greedy.
However, you can leave the \n after you have corrected the
character class to "[ \t]" - it just means that
the \n will be part of the match, but still allow
for the possibility that the last line hits the end
of the buffer.

Ahhhh, regular expressions.  I think in my entire history
as a programmer, learning about regular expressions was
the biggest braintrip I ever had - still love them.

> 
> to get the desired results:
> 
>    ,---------------------------------
>    |  43::PROPERTIES:
>    |    ::CUSTOM_ID: Setup
>    |    ::END:
>    | 131::PROPERTIES:
>    |    ::CUSTOM_ID: OrgFiles
>    |    ::END:
>    | 185::PROPERTIES:
>    |    ::CUSTOM_ID: AgendaSetup
>    |    ::END:
>    | 234::PROPERTIES:
>    |    ::CUSTOM_ID: OrgFileStructure
>    |    ::END:
>    `---------------------------------
> 
> A bug in the regexp?
> 
> PS 
> Can anybody explain this marvelous construct in the regexp:
> 
> ,---------
> | [^\\000]
> `---------

This is just a cheep way to match any character at all, because \000 should
not be part of any string (in C it indicates the end of a string).
In principle you could put any character you are sure will not turn up,
but \000 seems to be the safest choice.  It is
faster (I think) than "\\(.\\|\n\\)*" because the first will
just run fast and streight with a table lookup while the
latter need to always alternate between two alternatives.
I have not timed it, though.

> 
> I often pondered about how to achieve its effect with other means, since
> I did not find it in the Emacs Lisp manual.

There you go - sometimes a brain is better than the Emacs manual :)

Regards

- Carsten

> 
> -- 
> cheers,
> Thorsten
> 
> 


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [BUG] in org-property-drawer-re?

[Thorsten Jolitz]

Carsten Dominik <carsten.dominik@gmail.com> writes:

> On 1.10.2013, at 19:50, Thorsten Jolitz <tjolitz@gmail.com> wrote:
>
>>
>> Hi List,
>>
>> for the navi-mode keyword-search for complete property drawers I copied
>>
>> ,-----------------------
>> | org-property-drawer-re
>> `-----------------------
>>
>> from org.el:
>>
>> #+begin_src emacs-lisp
>>  (concat "\\(" org-property-start-re "\\)[^\000]*\\("
>> 	  org-property-end-re "\\)\n?")
>> #+end_src
>>
>> #+results:
>> : \(^[ 	]*:PROPERTIES:[ 	]*$\)[^\\000]*\(^[ 	]*:END:[ 	]*$\)
>> : ?
>>
>> A bit unreadable, but you get the message ... here is my hopefully equivalent
>> version:
>>
>> ,--------------------------------------------------------------
>> | (:propertydrawer
>> |   . (concat "\\(^[\\s\\t]*:PROPERTIES:[\\s\\t]*$\\)[^\\000]*"
>> |             "\\(^[\\s\\t]*:END:[\\s\\t]*$\\)\\n?"))
>> `--------------------------------------------------------------
>>
>> But this did not match correctly in Bernt Hansens tutorial:
>
> Indeed, this is a bad regular expression, it is too greedy and will
> match all the way to the last :END: line it can find.  also, \\s is
> wrong, it should be just a space, so "[ \t]". Luckily
> this regular expression does not seem to be used in Org as far
> as I can see....


But, if this is equivalent to the #+results: block above, it is defined
in org.el without that one ? indicated that makes the difference:

,-----------------------------------------------------------
| (:propertydrawer
|   . (concat "\\(^[ \\t]*:PROPERTIES:[ \\t]*$\\)[^\\000]*?" <=
|             "\\(^[ \\t]*:END:[ \\t]*$\\)\\n?"))
`-----------------------------------------------------------

> Yes, you need the star to make it non-greedy.

the '?' I guess ...

> However, you can leave the \n after you have corrected the
> character class to "[ \t]" - it just means that
> the \n will be part of the match, but still allow
> for the possibility that the last line hits the end
> of the buffer.

ok, I see

> Ahhhh, regular expressions.  I think in my entire history
> as a programmer, learning about regular expressions was
> the biggest braintrip I ever had - still love them.

thanks god for M-x regexp-builder ;)

>> PS
>> Can anybody explain this marvelous construct in the regexp:
>>
>> ,---------
>> | [^\\000]
>> `---------
>
> This is just a cheep way to match any character at all, because \000 should
> not be part of any string (in C it indicates the end of a string).
> In principle you could put any character you are sure will not turn up,
> but \000 seems to be the safest choice.  It is
> faster (I think) than "\\(.\\|\n\\)*" because the first will
> just run fast and streight with a table lookup while the
> latter need to always alternate between two alternatives.
> I have not timed it, though.

This is a very nice trick, and the alternative looks easy too - I have
to remember this.

>> I often pondered about how to achieve its effect with other means, since
>> I did not find it in the Emacs Lisp manual.
>
> There you go - sometimes a brain is better than the Emacs manual :)

Thanks a lot

--
cheers,
Thorsten

Re: [BUG] in org-property-drawer-re?

[Carsten Dominik]

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


On 1.10.2013, at 20:36, Thorsten Jolitz <tjolitz@gmail.com> wrote:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> On 1.10.2013, at 19:50, Thorsten Jolitz <tjolitz@gmail.com> wrote:
>> 
>>> 
>>> Hi List,
>>> 
>>> for the navi-mode keyword-search for complete property drawers I copied
>>> 
>>> ,-----------------------
>>> | org-property-drawer-re
>>> `-----------------------
>>> 
>>> from org.el:
>>> 
>>> #+begin_src emacs-lisp
>>> (concat "\\(" org-property-start-re "\\)[^\000]*\\("
>>> 	  org-property-end-re "\\)\n?")
>>> #+end_src
>>> 
>>> #+results:
>>> : \(^[ 	]*:PROPERTIES:[ 	]*$\)[^\\000]*\(^[ 	]*:END:[ 	]*$\)
>>> : ?
>>> 
>>> A bit unreadable, but you get the message ... here is my hopefully equivalent
>>> version:
>>> 
>>> ,--------------------------------------------------------------
>>> | (:propertydrawer
>>> |   . (concat "\\(^[\\s\\t]*:PROPERTIES:[\\s\\t]*$\\)[^\\000]*"
>>> |             "\\(^[\\s\\t]*:END:[\\s\\t]*$\\)\\n?"))
>>> `--------------------------------------------------------------
>>> 
>>> But this did not match correctly in Bernt Hansens tutorial:
>> 
>> Indeed, this is a bad regular expression, it is too greedy and will
>> match all the way to the last :END: line it can find.  also, \\s is
>> wrong, it should be just a space, so "[ \t]". Luckily
>> this regular expression does not seem to be used in Org as far
>> as I can see....
> 
> 
> But, if this is equivalent to the #+results: block above, it is defined
> in org.el without that one ? indicated that makes the difference:
> 
> ,-----------------------------------------------------------
> | (:propertydrawer
> |   . (concat "\\(^[ \\t]*:PROPERTIES:[ \\t]*$\\)[^\\000]*?" <=
> |             "\\(^[ \\t]*:END:[ \\t]*$\\)\\n?"))
> `-----------------------------------------------------------

Yes.  this is a bug, fortunately inconsequential since org does
its property matching in a different way.

Anyway, this bug is fixed in master.

- Carsten

> 
>> Yes, you need the star to make it non-greedy.
> 
> the '?' I guess ...
> 
>> However, you can leave the \n after you have corrected the
>> character class to "[ \t]" - it just means that
>> the \n will be part of the match, but still allow
>> for the possibility that the last line hits the end
>> of the buffer.
> 
> ok, I see
> 
>> Ahhhh, regular expressions.  I think in my entire history
>> as a programmer, learning about regular expressions was
>> the biggest braintrip I ever had - still love them.
> 
> thanks god for M-x regexp-builder ;)
> 
>>> PS
>>> Can anybody explain this marvelous construct in the regexp:
>>> 
>>> ,---------
>>> | [^\\000]
>>> `---------
>> 
>> This is just a cheep way to match any character at all, because \000 should
>> not be part of any string (in C it indicates the end of a string).
>> In principle you could put any character you are sure will not turn up,
>> but \000 seems to be the safest choice.  It is
>> faster (I think) than "\\(.\\|\n\\)*" because the first will
>> just run fast and streight with a table lookup while the
>> latter need to always alternate between two alternatives.
>> I have not timed it, though.
> 
> This is a very nice trick, and the alternative looks easy too - I have
> to remember this.
> 
>>> I often pondered about how to achieve its effect with other means, since
>>> I did not find it in the Emacs Lisp manual.
>> 
>> There you go - sometimes a brain is better than the Emacs manual :)
> 
> Thanks a lot
> 
> --
> cheers,
> Thorsten
> 
> 


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [BUG] in org-property-drawer-re?

[Nicolas Goaziou]

Hello,

Carsten Dominik <carsten.dominik@gmail.com> writes:

> This is just a cheep way to match any character at all, because \000 should
> not be part of any string (in C it indicates the end of a string).
> In principle you could put any character you are sure will not turn up,
> but \000 seems to be the safest choice.  It is
> faster (I think) than "\\(.\\|\n\\)*" because the first will
> just run fast and streight with a table lookup while the
> latter need to always alternate between two alternatives.
> I have not timed it, though.

On that topic, I would add that "^\000" must be used with care, as it
can lead to a stack overflow in regexp matcher error quite easily. In
particular, it may be safe to use it to match a property drawer, which
will not be very large, but I think it's wrong to use it to match
regular blocks or drawers, which can have arbitrary long size.

For example a regexp like "[^\000]\\." will fail when matching around
500 lines (72 characters long). Of course, constructs like
"\\(.\\|\n\\)*\\." will also fail, but my point is that it is tempting
to use "^\000" even though a regexp may not be the correct answer to the
problem.


Regards,

-- 
Nicolas Goaziou

Re: [BUG] in org-property-drawer-re?

[Carsten Dominik]

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


On Oct 2, 2013, at 11:55 AM, Nicolas Goaziou <n.goaziou@gmail.com> wrote:

> Hello,
> 
> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> This is just a cheep way to match any character at all, because \000 should
>> not be part of any string (in C it indicates the end of a string).
>> In principle you could put any character you are sure will not turn up,
>> but \000 seems to be the safest choice.  It is
>> faster (I think) than "\\(.\\|\n\\)*" because the first will
>> just run fast and streight with a table lookup while the
>> latter need to always alternate between two alternatives.
>> I have not timed it, though.
> 
> On that topic, I would add that "^\000" must be used with care, as it
> can lead to a stack overflow in regexp matcher error quite easily. In
> particular, it may be safe to use it to match a property drawer, which
> will not be very large, but I think it's wrong to use it to match
> regular blocks or drawers, which can have arbitrary long size.
> 
> For example a regexp like "[^\000]\\." will fail when matching around
> 500 lines (72 characters long). Of course, constructs like
> "\\(.\\|\n\\)*\\." will also fail, but my point is that it is tempting
> to use "^\000" even though a regexp may not be the correct answer to the
> problem.

Yes, I agree.  This is why the real matching Org does is first looking for
 a begin line, and then for the END line, in two independent searches.
Much better and safer.

- Carsten

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [BUG] in org-property-drawer-re?

[Nicolas Goaziou]

Carsten Dominik <carsten.dominik@gmail.com> writes:

> This is why the real matching Org does is first looking for a begin
> line, and then for the END line, in two independent searches.

Not always: see `org-block-regexp' and `org-babel-src-block-regexp', for
example.


Regards,

-- 
Nicolas Goaziou

Re: [BUG] in org-property-drawer-re?

[Carsten Dominik]

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


On Oct 2, 2013, at 1:14 PM, Nicolas Goaziou <n.goaziou@gmail.com> wrote:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> This is why the real matching Org does is first looking for a begin
>> line, and then for the END line, in two independent searches.
> 
> Not always: see `org-block-regexp' and `org-babel-src-block-regexp', for
> example.

I see.  Well, I guess so far we have not run into trouble, so we can save changing this of another time.

- Carsten

> 
> 
> Regards,
> 
> -- 
> Nicolas Goaziou


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [BUG] in org-property-drawer-re?

[Thorsten Jolitz]

Nicolas Goaziou <n.goaziou@gmail.com> writes:

Hello,

> Carsten Dominik <carsten.dominik@gmail.com> writes:
>
>> This is just a cheep way to match any character at all, because \000 should
>> not be part of any string (in C it indicates the end of a string).
>> In principle you could put any character you are sure will not turn up,
>> but \000 seems to be the safest choice.  It is
>> faster (I think) than "\\(.\\|\n\\)*" because the first will
>> just run fast and streight with a table lookup while the
>> latter need to always alternate between two alternatives.
>> I have not timed it, though.
>
> On that topic, I would add that "^\000" must be used with care, as it
> can lead to a stack overflow in regexp matcher error quite easily. In
> particular, it may be safe to use it to match a property drawer, which
> will not be very large, but I think it's wrong to use it to match
> regular blocks or drawers, which can have arbitrary long size.
>
> For example a regexp like "[^\000]\\." will fail when matching around
> 500 lines (72 characters long). Of course, constructs like
> "\\(.\\|\n\\)*\\." will also fail, but my point is that it is tempting
> to use "^\000" even though a regexp may not be the correct answer to the
> problem.

Conceptually it looks perfect(ly simple) to me, and I'm happy that I
discovered the pattern now by accident (better late than never), but
stack overflow is of course an important point.

Reminds me of my little [[https://github.com/tj64/org-hlc][org-hlc.el]]
library (hidden lines cookies)

,-------------------------------------------------------------------------------
| org-hlc.el implements hidden-lines-cookies for Org-mode
|
| hidden-lines-cookies (hlc) are small cookies at the end of each folded (and
| visible) headline in an Org-mode buffer that show the number of hidden lines
| before the next visible headline.
|
| hidden-lines-cookies can be handled with three user commands:
| org-hlc-show-hidden-lines-cookies, org-hlc-hide-hidden-lines-cookies, and the
| convenience command org-hlc-toggle-hidden-lines-cookies that toggles between
| the two other commands conditional on the last one executed.
|
| The appearance of the cookies can be customized by changing the values of four
| customizable variables: org-hlc-hidden-lines-cookie-left-delimiter (with
| default value "["), org-hlc-hidden-lines-cookie-right-delimiter (with default
| value "]"), org-hlc-hidden-lines-cookie-left-signal-char (with default value
| "#") and org-hlc-hidden-lines-cookie-right-signal-char (with default value
| "").
|
| Thus an exemplary folded headline with 165 hidden lines before the next
| visible headline might look like this when hidden-lines-cookies are shown:
|
| ,----------------- | *** Headline [#165] `-----------------
|
`-------------------------------------------------------------------------------

that was conceptually all 'wrong':

 - the cookies are written in the original buffer (but that could easily be
   changed to a temporary indirect buffer)
 - cookies are only shown/updated with explicit user commands

but it works even in 7000 lines .el or .org files. 

The other attempt to 'do it right' based on a library by Francois
Pinard, [[https://github.com/tj64/org-weights][org-weights.el]], uses
overlays and dynamic updates of the tree-weights via change hooks - much
better in theory - but somehow overwhelms Emacs capacities in big files
so that user experience is affected.

--
cheers,
Thorsten