Re: [PATCH] lisp/org-agenda.el: Fix filter preset problem for sticky agenda

[Liu Hui]

Ihor Radchenko  writes:

> Applied onto main with minor amendments (mostly added double space " "
> between sentences in docstrings and the commit message).
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=edf5afc1d833a814cf97e814194b83b82f26a294

Thanks!

> Also, while we are at it, may you take a look at
> https://list.orgmode.org/paxpr06mb7760f3a02d140e7de2baaec6c6...@paxpr06mb7760.eurprd06.prod.outlook.com/?
> AFAIR, it is caused by a similar symbol property approach employed in
> different place in org-agenda.el.

The issue seems related to other problem about the agenda restriction
rather than the use of symbol property. I will reply in other threads.

Re: [PATCH] lisp/org-agenda.el: Fix filter preset problem for sticky agenda

[Ihor Radchenko]

Liu Hui  writes:

> When writing the test, I find the original patch only addresses the
> case where an entry contains multiple commands. Now the udpated patch
> can address the case containing one command by changing another global
> symbol property to per-buffer text property.
>
> From 718cb5258a407d8a51eb4a5bac3d0c8025a3f198 Mon Sep 17 00:00:00 2001
> From: Liu Hui 
> Date: Tue, 4 Oct 2022 11:12:41 +0800
> Subject: [PATCH] Fix filter preset problem for sticky agenda

Applied onto main with minor amendments (mostly added double space " "
between sentences in docstrings and the commit message).
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=edf5afc1d833a814cf97e814194b83b82f26a294

Thanks for your contribution!

Also, while we are at it, may you take a look at
https://list.orgmode.org/paxpr06mb7760f3a02d140e7de2baaec6c6...@paxpr06mb7760.eurprd06.prod.outlook.com/?
AFAIR, it is caused by a similar symbol property approach employed in
different place in org-agenda.el.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/org-agenda.el: Fix filter preset problem for sticky agenda

[Liu Hui]

Ihor Radchenko  writes:

>> +(defvar org-agenda-filters-preset nil
>> +  "Preset of filters, which becomes buffer-local in org-agenda buffers.")
>
> Can you detail the value format in the docstring?
> It would also be useful to mention `org-agenda-local-vars'. Otherwise,
> the reader might be confused how `defvar' is buffer-local.

Done.

> Also, may you create a test for the reported bug in
> testing/lisp/test-org-agenda.el?

Done.

When writing the test, I find the original patch only addresses the
case where an entry contains multiple commands. Now the udpated patch
can address the case containing one command by changing another global
symbol property to per-buffer text property.

>From 718cb5258a407d8a51eb4a5bac3d0c8025a3f198 Mon Sep 17 00:00:00 2001
From: Liu Hui 
Date: Tue, 4 Oct 2022 11:12:41 +0800
Subject: [PATCH] Fix filter preset problem for sticky agenda

* lisp/org-agenda.el (org-agenda-local-vars):
(org-agenda-filters-preset): Add a new variable
`org-agenda-filters-preset' for storing per-buffer filter presets.
(org-agenda):
(org-agenda-filter-any):
(org-agenda-prepare):
(org-agenda-finalize):
(org-agenda-redo):
(org-agenda-filter-by-tag):
(org-agenda-filter-make-matcher):
(org-agenda-set-mode-name):
(org-agenda-reapply-filters): Use `org-agenda-filters-preset' for
getting and setting per-buffer filter presets, rather than modifying
the global symbol property. Change `org-lprops' from symbol property
to per-buffer text property. Delete unused `last-args' symbol
property.
* testing/lisp/test-org-agenda.el (test-org-agenda/sticky-agenda-filter-preset):
(test-org-agenda/redo-setting): add tests.
---
 lisp/org-agenda.el  | 108 +++-
 testing/lisp/test-org-agenda.el |  60 ++
 2 files changed, 109 insertions(+), 59 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index e5df768ff..c303aead1 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -2276,6 +2276,7 @@ When nil, `q' will kill the single agenda buffer."
 org-agenda-top-headline-filter
 org-agenda-regexp-filter
 org-agenda-effort-filter
+org-agenda-filters-preset
 org-agenda-markers
 org-agenda-last-search-view-search-was-boolean
 org-agenda-last-indirect-buffer
@@ -2929,10 +2930,6 @@ Pressing `<' twice means to restrict to the current subtree or region
 	(setq org-agenda-restrict nil)
 	(move-marker org-agenda-restrict-begin nil)
 	(move-marker org-agenda-restrict-end nil))
-  ;; Delete old local properties
-  (put 'org-agenda-redo-command 'org-lprops nil)
-  ;; Delete previously set last-arguments
-  (put 'org-agenda-redo-command 'last-args nil)
   ;; Remember where this call originated
   (setq org-agenda-last-dispatch-buffer (current-buffer))
   (unless org-keys
@@ -2981,7 +2978,6 @@ Pressing `<' twice means to restrict to the current subtree or region
 		(setq org-agenda-buffer-name
 		  (or (and (stringp org-match) (format "*Org Agenda(%s:%s)*" org-keys org-match))
 			  (format "*Org Agenda(%s)*" org-keys
-	  (put 'org-agenda-redo-command 'org-lprops lprops)
 	  (cl-progv
 	  (mapcar #'car lprops)
 	  (mapcar (lambda (binding) (eval (cadr binding) t)) lprops)
@@ -3016,7 +3012,10 @@ Pressing `<' twice means to restrict to the current subtree or region
 		   (funcall type org-match))
 		  ;; FIXME: Will signal an error since it's not `functionp'!
 		  ((pred fboundp) (funcall type org-match))
-		  (_ (user-error "Invalid custom agenda command type %s" type)
+		  (_ (user-error "Invalid custom agenda command type %s" type
+  (let ((inhibit-read-only t))
+	(add-text-properties (point-min) (point-max)
+			 `(org-lprops ,lprops
 	  (org-agenda-run-series (nth 1 entry) (cddr entry
((equal org-keys "C")
 	(setq org-agenda-custom-commands org-agenda-custom-commands-orig)
@@ -3808,6 +3807,10 @@ the entire agenda view.  In a block agenda, it will not work reliably to
 define a filter for one of the individual blocks.  You need to set it in
 the global options and expect it to be applied to the entire view.")
 
+(defvar org-agenda-filters-preset nil
+  "Alist of filter types and associated preset of filters.
+This variable is local in org-agenda buffers. See `org-agenda-local-vars'.")
+
 (defconst org-agenda-filter-variables
   '((category . org-agenda-category-filter)
 (tag . org-agenda-tag-filter)
@@ -3818,7 +3821,7 @@ the global options and expect it to be applied to the entire view.")
   "Is any filter active?"
   (cl-some (lambda (x)
 	 (or (symbol-value (cdr x))
-		 (get :preset-filter x)))
+ (assoc-default (car x) org-agenda-filters-preset)))
 	   org-agenda-filter-variables))
 
 (defvar org-agenda-category-filter-preset nil
@@ -3927,10 +3930,6 @@ FILTER-ALIST 

Re: [PATCH] lisp/org-agenda.el: Fix filter preset problem for sticky agenda

[Bastien Guerry]

Hi Ihor and Liu,

Ihor Radchenko  writes:

>>  I have signed the FSF
>> copyright assignment paper.
>
> Bastien, could you please confirm the FSF records?

I do confirm, I updated https://orgmode.org/worg/contributors.html

Liu, thanks in advance for contributing to Org!

Best,

-- 
 Bastien

Re: [PATCH] lisp/org-agenda.el: Fix filter preset problem for sticky agenda

[Ihor Radchenko]

Liu Hui  writes:

>  I have signed the FSF
> copyright assignment paper.

Bastien, could you please confirm the FSF records?

> This patch fixes the bug originally reported in
> https://list.orgmode.org/59e02fb6.1462370a.fffe8.5...@mx.google.com/
> with the following reproducing recipe. The bug still exists in latest
> org-mode.
> ...
>
> Because the filter presets are stored as symbol properties, they
> cannot be used with multiple sticky agenda buffers. The patch fixes
> the problem by adding a new variable `org-agenda-filters-preset' for
> getting and setting per-buffer filter presets. I have signed the FSF
> copyright assignment paper.

Thanks a lot for the contribution!

> Subject: [PATCH] lisp/org-agenda.el: Fix filter preset problem for sticky
>  agenda

The patch is generally good. I just have one minor comment and one
suggestion.

> +(defvar org-agenda-filters-preset nil
> +  "Preset of filters, which becomes buffer-local in org-agenda buffers.")

Can you detail the value format in the docstring?
It would also be useful to mention `org-agenda-local-vars'. Otherwise,
the reader might be confused how `defvar' is buffer-local.

Also, may you create a test for the reported bug in
testing/lisp/test-org-agenda.el?

-- 
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92

[PATCH] lisp/org-agenda.el: Fix filter preset problem for sticky agenda

[Liu Hui]

Hi,

This patch fixes the bug originally reported in
https://list.orgmode.org/59e02fb6.1462370a.fffe8.5...@mx.google.com/
with the following reproducing recipe. The bug still exists in latest
org-mode.

> To reproduce, take the following org file:
>
> ---8<---
> * Foo
>   :PROPERTIES:
>   :CATEGORY: foo
>   :END:
> ** TODO Foo todo one
>DEADLINE: <2017-10-13 Fri>
>- foo 1
>
> * Bar
>   :PROPERTIES:
>   :CATEGORY: bar
>   :END:
> ** TODO Bar todo one
>DEADLINE: <2017-10-13 Fri>
>- bar 1
> --->8---
>
> And the following elisp file to setup a minimal environment:
>
> ---8<---
> (require 'org)
> (setq org-agenda-files (list (expand-file-name "repro.org")))
> (setq org-agenda-sticky t)
> (setq org-agenda-custom-commands
>   '(("f" "foo"
>  ((agenda "" ())
>   (tags-todo "+CATEGORY=\"foo\"" ()))
>  ((org-agenda-category-filter-preset '("+foo"
>
> ("b" "bar"
>  ((agenda "" ())
>   (tags-todo "+CATEGORY=\"bar\"" ()))
>  ((org-agenda-category-filter-preset '("+bar"))
>
> (global-set-key (kbd "C-c a") #'org-agenda)
> --->8---
>
> Then run (something like):
>
> emacs -nw -Q -L ~/git/org-mode/lisp -l repro.el repro.org
>
> Followed by:
>
> C-c a f
> C-c a b
> C-x o
> r
>
> I would expect pressing `r' in the `*Org Agenda(f)*' buffer would
> keep the original `org-agenda-category-filter-preset' and that the
> preset in `*Org Agenda(b)*' should have no bearing on it.  But, as you
> can (probably) see from the mode line, the filter is set to "+bar", and
> refreshing the buffer causes all the TODOs to be lost because it now has
> the wrong filter.

Because the filter presets are stored as symbol properties, they
cannot be used with multiple sticky agenda buffers. The patch fixes
the problem by adding a new variable `org-agenda-filters-preset' for
getting and setting per-buffer filter presets. I have signed the FSF
copyright assignment paper.

Best,
Hui

>From 4fc03d86ab1df761ab26655f043b05b449b68716 Mon Sep 17 00:00:00 2001
From: Liu Hui 
Date: Tue, 4 Oct 2022 11:12:41 +0800
Subject: [PATCH] lisp/org-agenda.el: Fix filter preset problem for sticky
 agenda

* lisp/org-agenda.el (org-agenda-local-vars):
(org-agenda-filters-preset): Add a new variable
`org-agenda-filters-preset' for storing per-buffer filter presets.
(org-agenda-filter-any):
(org-agenda-prepare):
(org-agenda-finalize):
(org-agenda-redo):
(org-agenda-filter-by-tag):
(org-agenda-filter-make-matcher):
(org-agenda-set-mode-name):
(org-agenda-reapply-filters): Use `org-agenda-filters-preset' for
getting and setting per-buffer filter presets, rather than modifying
the global symbol property.
---
 lisp/org-agenda.el | 91 --
 1 file changed, 40 insertions(+), 51 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 2b56dd0fb..7ee0a6ad1 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -2276,6 +2276,7 @@ When nil, `q' will kill the single agenda buffer."
 org-agenda-top-headline-filter
 org-agenda-regexp-filter
 org-agenda-effort-filter
+org-agenda-filters-preset
 org-agenda-markers
 org-agenda-last-search-view-search-was-boolean
 org-agenda-last-indirect-buffer
@@ -3808,6 +3809,9 @@ the entire agenda view.  In a block agenda, it will not work reliably to
 define a filter for one of the individual blocks.  You need to set it in
 the global options and expect it to be applied to the entire view.")
 
+(defvar org-agenda-filters-preset nil
+  "Preset of filters, which becomes buffer-local in org-agenda buffers.")
+
 (defconst org-agenda-filter-variables
   '((category . org-agenda-category-filter)
 (tag . org-agenda-tag-filter)
@@ -3818,7 +3822,7 @@ the global options and expect it to be applied to the entire view.")
   "Is any filter active?"
   (cl-some (lambda (x)
 	 (or (symbol-value (cdr x))
-		 (get :preset-filter x)))
+ (assoc-default (car x) org-agenda-filters-preset)))
 	   org-agenda-filter-variables))
 
 (defvar org-agenda-category-filter-preset nil
@@ -3927,10 +3931,6 @@ FILTER-ALIST is an alist of filters we need to apply when
 			(cat . ,org-agenda-category-filter))
 (if (org-agenda-use-sticky-p)
 	(progn
-	  (put 'org-agenda-tag-filter :preset-filter nil)
-	  (put 'org-agenda-category-filter :preset-filter nil)
-	  (put 'org-agenda-regexp-filter :preset-filter nil)
-	  (put 'org-agenda-effort-filter :preset-filter nil)
 	  ;; Popup existing buffer
 	  (org-agenda-prepare-window (get-buffer org-agenda-buffer-name)
  filter-alist)
@@ -3938,14 +3938,6 @@