emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Possibly Bug in function org-scan-tags
@ 2011-12-15  1:18 bala subramaniyam
  2011-12-15 22:56 ` bala mayam
  2011-12-31  8:29 ` Carsten Dominik
  0 siblings, 2 replies; 6+ messages in thread
From: bala subramaniyam @ 2011-12-15  1:18 UTC (permalink / raw)
  To: emacs-orgmode

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

Hi,
             The variable "org-map-continue-from" is not reset to nil after
the funcall to action in function "org-scan-tags".

Heres the patch which works

diff --git a/lisp/org.el b/lisp/org.el
index 8a1fbd3..54ab5fb 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12848,7 +12848,8 @@ only lines with a TODO keyword are included in the
output."
              (setq org-map-continue-from nil)
              (save-excursion
                (setq rtn1 (funcall action))
-               (push rtn1 rtn)))
+               (push rtn1 rtn))
+             (setq org-map-continue-from nil))
             (t (error "Invalid action")))

            ;; if we are to skip sublevels, jump to end of subtree

To see the error in action assume that we want to archive all the "DONE"
states in the file
/tmp/test.org which contains the following lines.
* s1
** DONE ss1
** NEXT ss2


(defun my-org-archive-subtree ()
  (setq org-map-continue-from (point-at-bol))
  (org-archive-subtree))

 (org-map-entries 'my-org-archive-subtree "/DONE" (list "/tmp/test.org"))

While the org-scan-tags funciton parses the first subheading(**DONE ss1)
the match is
successful and the function "my-org-archive-subtree" is called which sets
the variable
"org-map-continue-from" value to *beginning of line* and this variable is
not set back to nil after the function call ends, which leads to infinite
loop while parsing the subsequent headlines which does not match the "DONE"
state(**NEXT ss2).

Regards,
Balamayam

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Possibly Bug in function org-scan-tags
  2011-12-15  1:18 Possibly Bug in function org-scan-tags bala subramaniyam
@ 2011-12-15 22:56 ` bala mayam
  2011-12-16  6:41   ` Nick Dokos
  2011-12-16 23:05   ` Nick Dokos
  2011-12-31  8:29 ` Carsten Dominik
  1 sibling, 2 replies; 6+ messages in thread
From: bala mayam @ 2011-12-15 22:56 UTC (permalink / raw)
  To: emacs-orgmode

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

The patch has error, it does not work for the following example file
/tmp/example.org
* DONE s1
* DONE s2
* DONE s3

The below patch works for all the cases

diff --git a/lisp/org.el b/lisp/org.el
index 8a1fbd3..93d603f 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12858,7 +12858,8 @@ only lines with a TODO keyword are included in the
outpu
        ;; Get the correct position from where to continue
        (if org-map-continue-from
            (goto-char org-map-continue-from)
-         (and (= (point) lspos) (end-of-line 1)))))
+         (and (= (point) lspos) (end-of-line 1)))
+       (setq org-map-continue-from nil)))
     (when (and (eq action 'sparse-tree)
               (not org-sparse-tree-open-archived-trees))
       (org-hide-archived-subtrees (point-min) (point-max)))

- Balamayam

On Thu, Dec 15, 2011 at 3:18 AM, bala subramaniyam <balamayam@gmail.com>wrote:

> Hi,
>              The variable "org-map-continue-from" is not reset to nil
> after the funcall to action in function "org-scan-tags".
>
> Heres the patch which works
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 8a1fbd3..54ab5fb 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -12848,7 +12848,8 @@ only lines with a TODO keyword are included in the
> output."
>               (setq org-map-continue-from nil)
>               (save-excursion
>                 (setq rtn1 (funcall action))
> -               (push rtn1 rtn)))
> +               (push rtn1 rtn))
> +             (setq org-map-continue-from nil))
>              (t (error "Invalid action")))
>
>             ;; if we are to skip sublevels, jump to end of subtree
>
> To see the error in action assume that we want to archive all the "DONE"
> states in the file
> /tmp/test.org which contains the following lines.
> * s1
> ** DONE ss1
> ** NEXT ss2
>
>
> (defun my-org-archive-subtree ()
>   (setq org-map-continue-from (point-at-bol))
>   (org-archive-subtree))
>
>  (org-map-entries 'my-org-archive-subtree "/DONE" (list "/tmp/test.org"))
>
> While the org-scan-tags funciton parses the first subheading(**DONE ss1)
> the match is
> successful and the function "my-org-archive-subtree" is called which sets
> the variable
> "org-map-continue-from" value to *beginning of line* and this variable is
> not set back to nil after the function call ends, which leads to infinite
> loop while parsing the subsequent headlines which does not match the "DONE"
> state(**NEXT ss2).
>
> Regards,
> Balamayam
>

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Possibly Bug in function org-scan-tags
  2011-12-15 22:56 ` bala mayam
@ 2011-12-16  6:41   ` Nick Dokos
  2011-12-16  7:16     ` Nick Dokos
  2011-12-16 23:05   ` Nick Dokos
  1 sibling, 1 reply; 6+ messages in thread
From: Nick Dokos @ 2011-12-16  6:41 UTC (permalink / raw)
  To: bala mayam; +Cc: nicholas.dokos, emacs-orgmode

bala mayam <balamayam@gmail.com> wrote:

> The patch has error, it does not work for the following example file
> /tmp/example.org
> * DONE s1
> * DONE s2
> * DONE s3
> 
> The below patch works for all the cases
> 
> diff --git a/lisp/org.el b/lisp/org.el
> index 8a1fbd3..93d603f 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -12858,7 +12858,8 @@ only lines with a TODO keyword are included in the outpu
>         ;; Get the correct position from where to continue
>         (if org-map-continue-from
>             (goto-char org-map-continue-from)
> -         (and (= (point) lspos) (end-of-line 1)))))
> +         (and (= (point) lspos) (end-of-line 1)))
> +       (setq org-map-continue-from nil)))
>      (when (and (eq action 'sparse-tree)
>                (not org-sparse-tree-open-archived-trees))
>        (org-hide-archived-subtrees (point-min) (point-max)))
> 
> - Balamayam
> 
> On Thu, Dec 15, 2011 at 3:18 AM, bala subramaniyam <balamayam@gmail.com> wrote:
> 
>     Hi,
>                  The variable "org-map-continue-from" is not reset to nil after the funcall to action
>     in function "org-scan-tags".
>    
>     Heres the patch which works
>    
>     diff --git a/lisp/org.el b/lisp/org.el
>     index 8a1fbd3..54ab5fb 100644
>     --- a/lisp/org.el
>     +++ b/lisp/org.el
>     @@ -12848,7 +12848,8 @@ only lines with a TODO keyword are included in the output."
>                   (setq org-map-continue-from nil)
>                   (save-excursion
>                     (setq rtn1 (funcall action))
>     -               (push rtn1 rtn)))
>     +               (push rtn1 rtn))
>     +             (setq org-map-continue-from nil))
>                  (t (error "Invalid action")))
>      
>                 ;; if we are to skip sublevels, jump to end of subtree
>    
>     To see the error in action assume that we want to archive all the "DONE" states in the file
>     /tmp/test.org which contains the following lines.
>     * s1
>     ** DONE ss1
>     ** NEXT ss2
> 
>     (defun my-org-archive-subtree ()
>       (setq org-map-continue-from (point-at-bol))
>       (org-archive-subtree))
>    
>      (org-map-entries 'my-org-archive-subtree "/DONE" (list "/tmp/test.org"))
>    
>     While the org-scan-tags funciton parses the first subheading(**DONE ss1) the match is
>     successful and the function "my-org-archive-subtree" is called which sets the variable
>     "org-map-continue-from" value to *beginning of line* and this variable is not set back to nil
>     after the function call ends, which leads to infinite loop while parsing the subsequent headlines
>     which does not match the "DONE" state(**NEXT ss2).
>    

I think it's a bit much to expect org to avoid all possible infinite loops
that a user function can get it into - it's up to you to write the function
properly so that it does *not* lead to infinite loops.

In this case, I think you can write

--8<---------------cut here---------------start------------->8---
(defun my-org-archive-subtree ()
  (let ((org-map-continue-from (point-at-bol)))
    (org-archive-subtree)))
--8<---------------cut here---------------end--------------->8---

and be done with it - no changes to org needed. AFAICT, it deals with
both of your examples without going into an infinite loop.

What am I missing?

Nick

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Possibly Bug in function org-scan-tags
  2011-12-16  6:41   ` Nick Dokos
@ 2011-12-16  7:16     ` Nick Dokos
  0 siblings, 0 replies; 6+ messages in thread
From: Nick Dokos @ 2011-12-16  7:16 UTC (permalink / raw)
  To: bala mayam, emacs-orgmode; +Cc: nicholas.dokos

Nick Dokos <nicholas.dokos@hp.com> wrote:

> >     While the org-scan-tags funciton parses the first subheading(**DONE ss1) the match is
> >     successful and the function "my-org-archive-subtree" is called which sets the variable
> >     "org-map-continue-from" value to *beginning of line* and this variable is not set back to nil
> >     after the function call ends, which leads to infinite loop while parsing the subsequent headlines
> >     which does not match the "DONE" state(**NEXT ss2).
> >    
> 
> I think it's a bit much to expect org to avoid all possible infinite loops
> that a user function can get it into - it's up to you to write the function
> properly so that it does *not* lead to infinite loops.
> 
> In this case, I think you can write
> 
> --8<---------------cut here---------------start------------->8---
> (defun my-org-archive-subtree ()
>   (let ((org-map-continue-from (point-at-bol)))
>     (org-archive-subtree)))
> --8<---------------cut here---------------end--------------->8---
> 
> and be done with it - no changes to org needed. AFAICT, it deals with
> both of your examples without going into an infinite loop.
> 
> What am I missing?
> 

Well, it doesn't go into an infinite loop but it doesn't work either,
so scratch that. I'll need to think about it some more.

Nick

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Possibly Bug in function org-scan-tags
  2011-12-15 22:56 ` bala mayam
  2011-12-16  6:41   ` Nick Dokos
@ 2011-12-16 23:05   ` Nick Dokos
  1 sibling, 0 replies; 6+ messages in thread
From: Nick Dokos @ 2011-12-16 23:05 UTC (permalink / raw)
  To: bala mayam; +Cc: nicholas.dokos, emacs-orgmode

bala mayam <balamayam@gmail.com> wrote:

> The patch has error, it does not work for the following example file
> /tmp/example.org
> * DONE s1
> * DONE s2
> * DONE s3
> 
> The below patch works for all the cases
> 
> diff --git a/lisp/org.el b/lisp/org.el
> index 8a1fbd3..93d603f 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -12858,7 +12858,8 @@ only lines with a TODO keyword are included in the outpu
>         ;; Get the correct position from where to continue
>         (if org-map-continue-from
>             (goto-char org-map-continue-from)
> -         (and (= (point) lspos) (end-of-line 1)))))
> +         (and (= (point) lspos) (end-of-line 1)))
> +       (setq org-map-continue-from nil)))
>      (when (and (eq action 'sparse-tree)
>                (not org-sparse-tree-open-archived-trees))
>        (org-hide-archived-subtrees (point-min) (point-max)))
> 

OK, I've looked at it some more and unless I'm mistaken again, I think
you are correct that it is a bug. Here's the structure of the relevant
code in org-scan-tags:

     ...
     (while (re-search-forward re nil t)
	(catch :skip
	   ...
	   (when ...
	      (cond
	         ((eq action 'sparse-tree)
		  ... do the sparse-tree stuff...)

		 ((eq action 'agenda)
		  ... do the agenda stuff ...)

		 ((functionp action)   ; user provided a function - the case here
                  (setq org-map-continue-from nil)
	          (save-excursion
		  	(setq rtn1 (funcall action))
			(push rtn1 rtn)))

		 (t (error "Invalid action")))

	      ... stuff after the cond ...
           ) ; closes the when form
	) ; closes the catch

	;; Get the correct position from where to continue
	(if org-map-continue-from
	    (goto-char org-map-continue-from)
	  (and (= (point) lspos) (end-of-line 1)))
    ) ; closes the while

    ...

So in the case of a user-supplied action, org-map-continue-from is
initialized to nil and then the function is called. The function (as in
your case) may modify org-map-continue-from which is then used at the
end of the while loop (and not reset to nil, as you observed, so next
time around, it will be non-nil even if we don't go through the
user-provided action).

I would fix it by pulling the (setq org-map-continue-from nil) from just
before the calling of the function all the way out to the top of the loop,
instead of adding another one at the end as you did, but that's a minor
point.

Nick

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Possibly Bug in function org-scan-tags
  2011-12-15  1:18 Possibly Bug in function org-scan-tags bala subramaniyam
  2011-12-15 22:56 ` bala mayam
@ 2011-12-31  8:29 ` Carsten Dominik
  1 sibling, 0 replies; 6+ messages in thread
From: Carsten Dominik @ 2011-12-31  8:29 UTC (permalink / raw)
  To: bala subramaniyam; +Cc: emacs-orgmode

I have just fixed this issue, in the spirit of Nicks proposal.

Thanks!

- Carsten

On 15.12.2011, at 02:18, bala subramaniyam wrote:

> Hi,
>              The variable "org-map-continue-from" is not reset to nil after the funcall to action in function "org-scan-tags".
> 
> Heres the patch which works
> 
> diff --git a/lisp/org.el b/lisp/org.el
> index 8a1fbd3..54ab5fb 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -12848,7 +12848,8 @@ only lines with a TODO keyword are included in the output."
>               (setq org-map-continue-from nil)
>               (save-excursion
>                 (setq rtn1 (funcall action))
> -               (push rtn1 rtn)))
> +               (push rtn1 rtn))
> +             (setq org-map-continue-from nil))
>              (t (error "Invalid action")))
>  
>             ;; if we are to skip sublevels, jump to end of subtree
> 
> To see the error in action assume that we want to archive all the "DONE" states in the file
> /tmp/test.org which contains the following lines.
> * s1
> ** DONE ss1
> ** NEXT ss2
> 
> 
> (defun my-org-archive-subtree ()
>   (setq org-map-continue-from (point-at-bol))
>   (org-archive-subtree))
> 
>  (org-map-entries 'my-org-archive-subtree "/DONE" (list "/tmp/test.org"))
> 
> While the org-scan-tags funciton parses the first subheading(**DONE ss1) the match is
> successful and the function "my-org-archive-subtree" is called which sets the variable
> "org-map-continue-from" value to *beginning of line* and this variable is not set back to nil after the function call ends, which leads to infinite loop while parsing the subsequent headlines which does not match the "DONE" state(**NEXT ss2).
> 
> Regards,
> Balamayam

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-12-31  8:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-15  1:18 Possibly Bug in function org-scan-tags bala subramaniyam
2011-12-15 22:56 ` bala mayam
2011-12-16  6:41   ` Nick Dokos
2011-12-16  7:16     ` Nick Dokos
2011-12-16 23:05   ` Nick Dokos
2011-12-31  8:29 ` Carsten Dominik

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).