[BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

This topic was brought up again in
https://lists.gnu.org/archive/html/emacs-orgmode/2022-05/msg00058.html,
I continue the discussion here:

>> Still, I think it might be interesting to compare this topic with the
>> one I linked in my reply,
>> https://lists.gnu.org/archive/html/emacs-orgmode/2022-03/msg00293.html,
>> which it's basically the same bug report but about COMMENT keywords. In
>> that regard, I have tested that org-capture targets do work regardless
>> of statistcs cookies. Could not something equivalent be done so that
>> they also work regardless of COMMENT keywords? Feel free to reply in
>> that other thread if you feel this is off-topic here.
>
>> This bug is related with the issue I reported in
>> https://lists.gnu.org/archive/html/emacs-orgmode/2022-03/msg00293.html. The
>> problem is that `org-heading-components' uses
>> `org-complex-heading-regexp', which does not consider statistics
>> cookies, and neither COMMENT keywords as I reported. I think it should be
>> updated to consider both.
>
> Note that org-complex-heading-regexp-format does consider statistics
> cookies, but only at the beginning/end of the headline title.
> Unfortunately, it is impossible to provide generic printf format to
> match a headline title with arbitrary statistics cookies inserted in the
> middle of it.
>

Could not something equivalent be done for COMMENT keywords and
optionally match them at the beginning of the headline in
`org-complex-heading-regexp-format'? Something like this:

  (setq org-complex-heading-regexp-format
        (concat "^\\(\\*+\\)"
  	      "\\(?: +" org-todo-regexp "\\)?"
  	      "\\(?: +\\(\\[#.\\]\\)\\)?"
  	      "\\(?: +"
+             ;; Headline might be commented
+             "\\(COMMENT \\)?
  	      ;; Stats cookies can be stuck to body.
  	      "\\(?:\\[[0-9%%/]+\\] *\\)*"
  	      "\\(%s\\)"
  	      "\\(?: *\\[[0-9%%/]+\\]\\)*"
  	      "\\)"
  	      "\\(?:[ \t]+\\(:[[:alnum:]_@#%%:]+:\\)\\)?"
  	      "[ \t]*$"))

This would fix the problem I really cared about: being able to use a
headline as capture target regardless of whether it is commented or
not. Getting the headline text was never important to me, so I don't
really care that much about `org-complex-heading-regexp'.

> As for your other report, it is a hard one - org-complex-heading-regexp
> is hard to modify because we guarantee certain match groups and its hard
> to fit COMMENT in there without breaking backward-compatibility.
>
> I generally dislike the idea of the available plethora of analytic
> regexps with numbered match groups.

Do you mean that there is no way to specify an optional string in a
regular expression without wrapping it in a parenthesized group, which
would break the match group numbering backwards compatibility? I'm not
that familiar with regular expressions, but if that's the case I
completely agree with your last statement.

--Ignacio

Re: [BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

>> Note that org-complex-heading-regexp-format does consider statistics
>> cookies, but only at the beginning/end of the headline title.
>> Unfortunately, it is impossible to provide generic printf format to
>> match a headline title with arbitrary statistics cookies inserted in the
>> middle of it.
>
> Could not something equivalent be done for COMMENT keywords and
> optionally match them at the beginning of the headline in
> `org-complex-heading-regexp-format'? Something like this:
>
>   (setq org-complex-heading-regexp-format
>         (concat "^\\(\\*+\\)"
>   	      "\\(?: +" org-todo-regexp "\\)?"
>   	      "\\(?: +\\(\\[#.\\]\\)\\)?"
>   	      "\\(?: +"
> +             ;; Headline might be commented
> +             "\\(COMMENT \\)?
>   	      ;; Stats cookies can be stuck to body.
>   	      "\\(?:\\[[0-9%%/]+\\] *\\)*"
>   	      "\\(%s\\)"
>   	      "\\(?: *\\[[0-9%%/]+\\]\\)*"
>   	      "\\)"
>   	      "\\(?:[ \t]+\\(:[[:alnum:]_@#%%:]+:\\)\\)?"
>   	      "[ \t]*$"))
>
> This would fix the problem I really cared about: being able to use a
> headline as capture target regardless of whether it is commented or
> not. Getting the headline text was never important to me, so I don't
> really care that much about `org-complex-heading-regexp'.

Sounds reasonable. Could you prepare a patch?
COMMENT should be inside a shy group and note that there might be an
arbitrary number of space after COMMENT string.

>> I generally dislike the idea of the available plethora of analytic
>> regexps with numbered match groups.
>
> Do you mean that there is no way to specify an optional string in a
> regular expression without wrapping it in a parenthesized group, which
> would break the match group numbering backwards compatibility? I'm not
> that familiar with regular expressions, but if that's the case I
> completely agree with your last statement.

Not exactly. We could use shy (unnumbered) match group and not break the
group numbers. However, adding an extra optional match means that we can
break an old code that could be relying on the next group to contain
this optional match:

Old code:
(when (string-match-p "COMMENT" (match-string 1)) ...)

Before "\\(.+\\)"
After "\\(?:COMMENT\\)?\\(.+\\)"

The old code will never match COMMENT when the new regexp is used.

Best,
Ihor

Re: [BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

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


> Sounds reasonable. Could you prepare a patch?
> COMMENT should be inside a shy group and note that there might be an
> arbitrary number of space after COMMENT string.

Here it is.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch --]
[-- Type: text/x-diff, Size: 774 bytes --]

From 54cd366c97bd64c226cc6fc79e125ee9f026ff66 Mon Sep 17 00:00:00 2001
From: Ignacio <ignaciocasso@hotmail.com>
Date: Fri, 6 May 2022 13:51:14 +0200
Subject: [PATCH] lisp/org.el: match COMMENT in
 `org-complex-heading-regexp-format'

---
 lisp/org.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index 1d5fc3903..0ec717322 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -4213,6 +4213,8 @@ related expressions."
 		      "\\(?: +" org-todo-regexp "\\)?"
 		      "\\(?: +\\(\\[#.\\]\\)\\)?"
 		      "\\(?: +"
+                      ;; Headline might be commented
+                      "\\(?:" org-comment-string " +\\)?"
 		      ;; Stats cookies can be stuck to body.
 		      "\\(?:\\[[0-9%%/]+\\] *\\)*"
 		      "\\(%s\\)"
-- 
2.25.1

Re: [BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

>> Sounds reasonable. Could you prepare a patch?
>> COMMENT should be inside a shy group and note that there might be an
>> arbitrary number of space after COMMENT string.
>
> Here it is.

Thanks! Could you also update the org-complex-heading-regexp-format
docstring and make the commit massage conform with
https://orgmode.org/worg/org-contribute.html#commit-messages (also see
https://orgmode.org/list/87ilqz14ys.fsf@localhost)?

Best,
Ihor

Re: [BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

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


Ihor Radchenko <yantar92@gmail.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>>> Sounds reasonable. Could you prepare a patch?
>>> COMMENT should be inside a shy group and note that there might be an
>>> arbitrary number of space after COMMENT string.
>>
>> Here it is.
>
> Thanks! Could you also update the org-complex-heading-regexp-format
> docstring and make the commit massage conform with
> https://orgmode.org/worg/org-contribute.html#commit-messages (also see
> https://orgmode.org/list/87ilqz14ys.fsf@localhost)?
>
> Best,
> Ihor

Done. I attach the patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch --]
[-- Type: text/x-diff, Size: 1517 bytes --]

From 646f2a792dc5c6e6fa3933eb1d06420c86cd53c3 Mon Sep 17 00:00:00 2001
From: Ignacio <ignaciocasso@hotmail.com>
Date: Fri, 6 May 2022 13:51:14 +0200
Subject: [PATCH] lisp/org.el: match COMMENT in
 `org-complex-heading-regexp-format'

* lisp/org.el (org-set-regexps-and-options): Change
`org-complex-heading-regexp-format' initialization so that the regexp
it produces also matches a headline if it is commented.
---
 lisp/org.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index 1d5fc3903..cab59b87c 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -3846,7 +3846,8 @@ expected to be bound to nil when matching against this regexp.")
   "Printf format to make regexp to match an exact headline.
 This regexp will match the headline of any node which has the
 exact headline text that is put into the format, but may have any
-TODO state, priority and tags.")
+TODO state, priority, tags, statistics cookies (at the beginning
+or end of the headline title), or COMMENT keyword.")
 
 (defvar-local org-todo-line-tags-regexp nil
   "Matches a headline and puts TODO state into group 2 if present.
@@ -4213,6 +4214,8 @@ related expressions."
 		      "\\(?: +" org-todo-regexp "\\)?"
 		      "\\(?: +\\(\\[#.\\]\\)\\)?"
 		      "\\(?: +"
+                      ;; Headline might be commented
+                      "\\(?:" org-comment-string " +\\)?"
 		      ;; Stats cookies can be stuck to body.
 		      "\\(?:\\[[0-9%%/]+\\] *\\)*"
 		      "\\(%s\\)"
-- 
2.25.1

Re: [BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> Done. I attach the patch:

Thanks!

Applied onto main as a3d9999d2 with minor amendments.
I have added TINYCHANGE cookie as you don't appear to have FSF copyright
assignment and capitalised "match" in the commit summary.

Best,
Ihor

> From 646f2a792dc5c6e6fa3933eb1d06420c86cd53c3 Mon Sep 17 00:00:00 2001
> From: Ignacio <ignaciocasso@hotmail.com>
> Date: Fri, 6 May 2022 13:51:14 +0200
> Subject: [PATCH] lisp/org.el: match COMMENT in
>  `org-complex-heading-regexp-format'
>
> * lisp/org.el (org-set-regexps-and-options): Change
> `org-complex-heading-regexp-format' initialization so that the regexp
> it produces also matches a headline if it is commented.
> ---
>  lisp/org.el | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 1d5fc3903..cab59b87c 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -3846,7 +3846,8 @@ expected to be bound to nil when matching against this regexp.")
>    "Printf format to make regexp to match an exact headline.
>  This regexp will match the headline of any node which has the
>  exact headline text that is put into the format, but may have any
> -TODO state, priority and tags.")
> +TODO state, priority, tags, statistics cookies (at the beginning
> +or end of the headline title), or COMMENT keyword.")
>  
>  (defvar-local org-todo-line-tags-regexp nil
>    "Matches a headline and puts TODO state into group 2 if present.
> @@ -4213,6 +4214,8 @@ related expressions."
>  		      "\\(?: +" org-todo-regexp "\\)?"
>  		      "\\(?: +\\(\\[#.\\]\\)\\)?"
>  		      "\\(?: +"
> +                      ;; Headline might be commented
> +                      "\\(?:" org-comment-string " +\\)?"
>  		      ;; Stats cookies can be stuck to body.
>  		      "\\(?:\\[[0-9%%/]+\\] *\\)*"
>  		      "\\(%s\\)"
> -- 
> 2.25.1
>

-- 
Ihor Radchenko,
PhD,
Center for Advancing Materials Performance from the Nanoscale (CAMP-nano)
State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China
Email: yantar92@gmail.com, ihor_radchenko@alumni.sutd.edu.sg

Re: [BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

> Thanks!

Thank you too!

> I have added TINYCHANGE cookie as you don't appear to have FSF copyright

I should have it, I did the paperwork for commit 2b564f504b in
Emacs. Maybe I used a different email? Or maybe the problem is that now
that I see it I don't have my full name configured in git. It's Ignacio
Casso, and I appear in the commit author field as just Ignacio. I have
fixed it now for future commits.

Re: [BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ihor Radchenko]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

>> I have added TINYCHANGE cookie as you don't appear to have FSF copyright
>
> I should have it, I did the paperwork for commit 2b564f504b in
> Emacs. Maybe I used a different email? Or maybe the problem is that now
> that I see it I don't have my full name configured in git. It's Ignacio
> Casso, and I appear in the commit author field as just Ignacio. I have
> fixed it now for future commits.

I see. The problem is that I do not have access to the up-to-date
contributor list. And you are not listed in
https://orgmode.org/worg/contributors.html

Bastien, could you kindly check if Ignacio Casso
<ignaciocasso@hotmail.com> has FSF assignment and update contributors
page accordingly?

Best,
Ihor

Re: [BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Bastien Guerry]

Hi Ihor and Ignacio,

sorry for the late answer.

Ihor Radchenko <yantar92@gmail.com> writes:

> Bastien, could you kindly check if Ignacio Casso
> <ignaciocasso@hotmail.com> has FSF assignment and update contributors
> page accordingly?

I confirm Ignacio has FSF assignment (since 2022-03-28) and I updated
the Org contributors page.

Best,

-- 
 Bastien

[BUG] org-complex-heading-regexp should consider COMMENT keywords [9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)]

[Ignacio Casso]

Hello,

I think `org-complex-heading-regexp' and
`org-complex-heading-regexp-format' should consider COMMENT keywords, as
they do with TODO keywords, priorities, and tags.


Firstly, with things as they are, different methods for accessing the
headline text are inconsistent. For example, for the headline below,
(org-entry-get (point) "ITEM"), which uses `org-complex-heading-regexp',
returns "COMMENT My headline", while (org-element-property :title
(org-element-context)), which doesn't, returns "My headline".

*** TODO [#A] COMMENT My headline   :my_tag:


But more importantly for me, this is a problem in particular with
org-capture, where I would like capture targets to be found regardless
of whether they are commented or not. But right now commented headlines
are only found if the COMMENT keyword also appears in the capture
template, and in that case if the headlines are later uncommented they
are no longer found. You can copy the entry below into an org file and
play around with it to reproduce this described behavior.

* COMMENT My Target
  #+begin_src emacs-lisp
    (let ((org-capture-templates
           `(("d" "My capture template" entry
              (file+headline ,buffer-file-name "My target")
              "* My content"
              :immediate-finish t))))
      (org-capture nil "d"))
  #+end_src

The cause of this is the use of `org-complex-heading-regexp-format' in
`org-capture-set-target-location'.

This is the bug I originally wanted to report, but after
exploring the cause I thought that the problem was really in
`org-complex-heading-regexp' and not org-capture's code.

What do you think? Do you know of any other uses of those variables and
how considering COMMENT keywords or not affects them?

If you agree I can send a patch that changes the default value of those
regexp variables. Otherwise, I can send a patch that just modifies the
code of `org-capture-set-target-location' to fix this only for the
org-capture use case.

Regards,

Ignacio


Emacs  : GNU Emacs 29.0.50 (build 13, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
 of 2022-03-21
Package: Org mode version 9.5.2 (release_9.5.2-25-gaf6f12 @ /home/ignacio/repos/emacs/lisp/org/)