emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Fix org-agenda-skip-if bug
@ 2012-02-12 21:06 Toby Cubitt
  2012-03-16 18:16 ` Bastien
  0 siblings, 1 reply; 2+ messages in thread
From: Toby Cubitt @ 2012-02-12 21:06 UTC (permalink / raw)
  To: emacs-orgmode

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

There appears to be a bug in how org-agenda-skip-if parses the list of
CONDITIONS supplied to it.

The combination '(nottodo todo) is a valid condition, matching todo items
whose state isn't a todo-type keyword (according to the keyword types
defined in `org-todo-keywords'). But `org-agenda-skip-if' tests first for
conditions of the form '(todo x) using (memq 'todo conditions), which
mistakenly picks up '(nottodo todo) as well.

Simply reversing the order of the memq tests for 'todo and 'nottodo fixes
this particular case, which is what the attached patch does.


Note that there's still a slightly different issue with combinations of
multiple todo tests, which this patch does not fix. The docstring
suggests that CONDITIONS is allowed to be a list of multiple
tests. E.g. '(nottodo CANCELLED todo done) should match any done state
except CANCELLED. But, faced with this combination,
`organ-agenda-skip-if' will only apply the first '(nottodo CANCELLED)
test, and ignores the second.

However, it's not clear to me whether this is a problem with the code or
the docstring. Perhaps it was never intended to support combinations of
multiple todo tests.

Toby
-- 
Dr T. S. Cubitt
Mathematics and Quantum Information group
Department of Mathematics
Complutense University
Madrid, Spain

email: tsc25@cantab.net
web:   www.dr-qubit.org

[-- Attachment #2: 0001-Agenda-Fix-bug-that-broke-nottodo-todo-skip-conditio.patch --]
[-- Type: text/plain, Size: 1038 bytes --]

From 3eb5cd877ca48a2c0d2e907cf4ab7adf32520020 Mon Sep 17 00:00:00 2001
From: Toby S. Cubitt <tsc25@cantab.net>
Date: Sat, 28 Jan 2012 18:21:52 +0100
Subject: [PATCH 1/2] Agenda: Fix bug that broke '(nottodo todo) skip condition

* lisp/org-agenda.el (org-agenda-skip-if): Fix bug in test for
'(nottodo todo) skip condition.

Note: certain combinations of multiple conditions may still be broken.
---
 lisp/org-agenda.el |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 38fd589..ad706eb 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -4285,8 +4285,8 @@ that can be put into `org-agenda-skip-function' for the duration of a command."
 	   (stringp (nth 1 m))
 	   (not (re-search-forward (nth 1 m) end t)))
       (and (or
-	    (setq m (memq 'todo conditions))
-	    (setq m (memq 'nottodo conditions)))
+	    (setq m (memq 'nottodo conditions))
+	    (setq m (memq 'todo conditions)))
 	   (org-agenda-skip-if-todo m end)))
      end)))
 
-- 
1.7.3.4


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

* Re: [PATCH] Fix org-agenda-skip-if bug
  2012-02-12 21:06 [PATCH] Fix org-agenda-skip-if bug Toby Cubitt
@ 2012-03-16 18:16 ` Bastien
  0 siblings, 0 replies; 2+ messages in thread
From: Bastien @ 2012-03-16 18:16 UTC (permalink / raw)
  To: emacs-orgmode

Hi Toby,

Toby Cubitt <tsc25@cantab.net> writes:

> There appears to be a bug in how org-agenda-skip-if parses the list of
> CONDITIONS supplied to it.

Applied, thanks.  

> The combination '(nottodo todo) is a valid condition, matching todo items
> whose state isn't a todo-type keyword (according to the keyword types
> defined in `org-todo-keywords'). But `org-agenda-skip-if' tests first for
> conditions of the form '(todo x) using (memq 'todo conditions), which
> mistakenly picks up '(nottodo todo) as well.
>
> Simply reversing the order of the memq tests for 'todo and 'nottodo fixes
> this particular case, which is what the attached patch does.

Thanks for the explanations.

> Note that there's still a slightly different issue with combinations of
> multiple todo tests, which this patch does not fix. The docstring
> suggests that CONDITIONS is allowed to be a list of multiple
> tests. E.g. '(nottodo CANCELLED todo done) should match any done state
> except CANCELLED. But, faced with this combination,
> `organ-agenda-skip-if' will only apply the first '(nottodo CANCELLED)
> test, and ignores the second.
>
> However, it's not clear to me whether this is a problem with the code or
> the docstring. Perhaps it was never intended to support combinations of
> multiple todo tests.

I don't know.  If someone can digg this issue further and report what
should be fixed, that'd help.

Thanks!

-- 
 Bastien

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

end of thread, other threads:[~2012-03-16 18:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-12 21:06 [PATCH] Fix org-agenda-skip-if bug Toby Cubitt
2012-03-16 18:16 ` Bastien

Code repositories for project(s) associated with this 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).