[Patch] Support for dimming local to each agenda

[Patch] Support for dimming local to each agenda

[Yuri Lensky]

[-- Attachment #1.1: Type: text/plain, Size: 73 bytes --]

Composite agenda views can now set org-agenda-dim-blocked-tasks locally.

[-- Attachment #1.2: Type: text/html, Size: 96 bytes --]

[-- Attachment #2: 0002-org-agenda.el-Support-for-dimming-local-to-each-agen.patch --]
[-- Type: application/octet-stream, Size: 3839 bytes --]

From 52f8bf79a198fa2e7f131c2a015a7c9400a403ac Mon Sep 17 00:00:00 2001
From: "Yuri D. Lensky" <ydlensky@gmail.com>
Date: Mon, 10 Jul 2017 19:21:39 -0700
Subject: [PATCH 2/2] org-agenda.el: Support for dimming local to each agenda.

Composite agenda views could not separately specify whether to dim
blocked tasks.
---
 lisp/org-agenda.el | 56 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 9ac4f65..f132420 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -3887,34 +3887,46 @@ dimming them."
       (delete-overlay o)))
   (save-excursion
     (let ((inhibit-read-only t)
-	  (org-depend-tag-blocked nil)
-	  org-blocked-by-checkboxes)
+	  (org-depend-tag-blocked nil))
       (goto-char (point-min))
       (while (let ((pos (text-property-not-all
-			 (point) (point-max) 'todo-state nil)))
+			 (point) (point-max) 'org-todo-blocked nil)))
 	       (when pos (goto-char pos)))
-	(setq org-blocked-by-checkboxes nil)
-	(let ((marker (org-get-at-bol 'org-hd-marker)))
-	  (when (and (markerp marker)
-		     (with-current-buffer (marker-buffer marker)
-		       (save-excursion (goto-char marker)
-				       (org-entry-blocked-p))))
-	    ;; Entries blocked by checkboxes cannot be made invisible.
-	    ;; See `org-agenda-dim-blocked-tasks' for details.
-	    (let* ((really-invisible
-		    (and (not org-blocked-by-checkboxes)
-			 (or invisible (eq org-agenda-dim-blocked-tasks
-					   'invisible))))
-		   (ov (make-overlay (if really-invisible (line-end-position 0)
-				       (line-beginning-position))
-				     (line-end-position))))
-	      (if really-invisible (overlay-put ov 'invisible t)
-		(overlay-put ov 'face 'org-agenda-dimmed-todo-face))
-	      (overlay-put ov 'org-type 'org-blocked-todo))))
+	(let* ((invisible (eq (org-get-at-bol 'org-todo-blocked) 'invisible))
+	       (ov (make-overlay (if invisible
+				     (line-end-position 0)
+				   (line-beginning-position))
+				 (line-end-position))))
+	  (if invisible
+	      (overlay-put ov 'invisible t)
+	    (overlay-put ov 'face 'org-agenda-dimmed-todo-face))
+	  (overlay-put ov 'org-type 'org-blocked-todo))
 	(forward-line))))
   (when (called-interactively-p 'interactive)
     (message "Dim or hide blocked tasks...done")))
 
+(defun org-agenda-mark-blocked-entry (entry)
+  "Mark a blocked entry in text properties."
+  (when (get-text-property 0 'todo-state entry)
+    (let ((entry-marker (get-text-property 0 'org-hd-marker entry))
+          (org-blocked-by-checkboxes nil)
+	  ;; Necessary so that org-entry-blocked-p does not change the buffer
+          (org-depend-tag-blocked nil))
+      (let ((blocked
+	     (with-current-buffer (marker-buffer entry-marker)
+	       (save-mark-and-excursion
+		 (goto-char entry-marker)
+		 (org-entry-blocked-p)))))
+	(when blocked
+	  (let ((really-invisible
+		 (and (not org-blocked-by-checkboxes)
+		      (eq org-agenda-dim-blocked-tasks 'invisible))))
+	    (put-text-property
+	     0 (length entry) 'org-todo-blocked
+	     (if really-invisible 'invisible t)
+	     entry)))))
+    entry))
+
 (defvar org-agenda-skip-function nil
   "Function to be called at each match during agenda construction.
 If this function returns nil, the current match should not be skipped.
@@ -6781,6 +6793,8 @@ The optional argument TYPE tells the agenda type."
       (setq list (org-agenda-limit-entries list 'tags max-tags)))
     (when max-entries
       (setq list (org-agenda-limit-entries list 'org-hd-marker max-entries)))
+    (when (and org-agenda-dim-blocked-tasks org-blocker-hook)
+      (setq list (mapcar 'org-agenda-mark-blocked-entry list)))
     (mapconcat 'identity list "\n")))
 
 (defun org-agenda-limit-entries (list prop limit &optional fn)
-- 
2.9.2.windows.1

Re: [Patch] Support for dimming local to each agenda

[Nicolas Goaziou]

Hello,

Yuri Lensky <ydl@ydl.cm> writes:

> Composite agenda views can now set org-agenda-dim-blocked-tasks locally.
> From 52f8bf79a198fa2e7f131c2a015a7c9400a403ac Mon Sep 17 00:00:00 2001
> From: "Yuri D. Lensky" <ydlensky@gmail.com>
> Date: Mon, 10 Jul 2017 19:21:39 -0700
> Subject: [PATCH 2/2] org-agenda.el: Support for dimming local to each
> agenda.

Thank you. Some comments follow.

> Composite agenda views could not separately specify whether to dim
> blocked tasks.

The commit message is lacking information about modified and created
functions, e.g.,

lisp/org-agenda.el (org-agenda-mark-blocked-entry): New function.
...

> +(defun org-agenda-mark-blocked-entry (entry)

Since this is meant to be an internal function, I suggest to name it
`org-agenda--mark-blocked-entry'.

> +  "Mark a blocked entry in text properties."

The docstring should specifiy what ENTRY is.

> +  (when (get-text-property 0 'todo-state entry)
> +    (let ((entry-marker (get-text-property 0 'org-hd-marker entry))
> +          (org-blocked-by-checkboxes nil)
> +	  ;; Necessary so that org-entry-blocked-p does not change the buffer

`org-entry-blocked-p'

Missing full stop, too.

> +          (org-depend-tag-blocked nil))
> +      (let ((blocked
> +	     (with-current-buffer (marker-buffer entry-marker)
> +	       (save-mark-and-excursion

Why is `save-mark-and-excursion' needed?

> +    (when (and org-agenda-dim-blocked-tasks org-blocker-hook)
> +      (setq list (mapcar 'org-agenda-mark-blocked-entry list)))

Nitpick: (mapcar #'org-agenda-mark-blocked-entry list)

Bonus points if you can write tests in test-org-agenda.el.

Regards,

-- 
Nicolas Goaziou

Re: [Patch] Support for dimming local to each agenda

[Yuri Lensky]

[-- Attachment #1.1: Type: text/plain, Size: 2226 bytes --]

Thanks. I have attached an updated patch that addresses your points (with
some additional cleanup).

On Thu, Jul 13, 2017 at 5:51 AM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Yuri Lensky <ydl@ydl.cm> writes:
>
> > Composite agenda views can now set org-agenda-dim-blocked-tasks locally.
> > From 52f8bf79a198fa2e7f131c2a015a7c9400a403ac Mon Sep 17 00:00:00 2001
> > From: "Yuri D. Lensky" <ydlensky@gmail.com>
> > Date: Mon, 10 Jul 2017 19:21:39 -0700
> > Subject: [PATCH 2/2] org-agenda.el: Support for dimming local to each
> > agenda.
>
> Thank you. Some comments follow.
>
> > Composite agenda views could not separately specify whether to dim
> > blocked tasks.
>
> The commit message is lacking information about modified and created
> functions, e.g.,
>
> lisp/org-agenda.el (org-agenda-mark-blocked-entry): New function.
> ...
>
Fixed.

>
> > +(defun org-agenda-mark-blocked-entry (entry)
>
> Since this is meant to be an internal function, I suggest to name it
> `org-agenda--mark-blocked-entry'.
>
Fixed.

>
> > +  "Mark a blocked entry in text properties."
>
> The docstring should specifiy what ENTRY is.
>
Fixed.

>
> > +  (when (get-text-property 0 'todo-state entry)
> > +    (let ((entry-marker (get-text-property 0 'org-hd-marker entry))
> > +          (org-blocked-by-checkboxes nil)
> > +       ;; Necessary so that org-entry-blocked-p does not change the
> buffer
>
> `org-entry-blocked-p'
>
> Missing full stop, too.
>
Fixed.

>
> > +          (org-depend-tag-blocked nil))
> > +      (let ((blocked
> > +          (with-current-buffer (marker-buffer entry-marker)
> > +            (save-mark-and-excursion
>
> Why is `save-mark-and-excursion' needed?
>
Because we are moving point to get to the heading, and in principle hooks
in org-blocker-hook can change point/mark.

>
> > +    (when (and org-agenda-dim-blocked-tasks org-blocker-hook)
> > +      (setq list (mapcar 'org-agenda-mark-blocked-entry list)))
>
> Nitpick: (mapcar #'org-agenda-mark-blocked-entry list)
>
Fixed.

>
> Bonus points if you can write tests in test-org-agenda.el.
>
Don't have time at the moment, but as this is a fairly simple feature it is
easy to see if it works.

>
> Regards,
>
> --
> Nicolas Goaziou

[-- Attachment #1.2: Type: text/html, Size: 3907 bytes --]

[-- Attachment #2: 0001-org-agenda.el-Support-for-dimming-local-to-each-agen.patch --]
[-- Type: application/octet-stream, Size: 4544 bytes --]

From 0d96dc827031b9e6598897bc5bce6216cba56e7e Mon Sep 17 00:00:00 2001
From: "Yuri D. Lensky" <ydlensky@gmail.com>
Date: Mon, 10 Jul 2017 19:21:39 -0700
Subject: [PATCH] org-agenda.el: Support for dimming local to each agenda.

Composite agenda views could not separately specify whether to dim
blocked tasks.

* lisp/org-agenda.el (org-agenda--mark-blocked-entry): New function
  that marks whether an entry (a string with text properties as passed
  around in agenda functions) is blocked according to
  org-entry-blocked-p.

  (org-agenda-dim-blocked-tasks): Modified to work with text
  properties set by org-agenda--marked-blocked-entry.
---
 lisp/org-agenda.el | 63 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 9ac4f65..5eabd57 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -3886,35 +3886,52 @@ dimming them."
     (when (eq (overlay-get o 'org-type) 'org-blocked-todo)
       (delete-overlay o)))
   (save-excursion
-    (let ((inhibit-read-only t)
-	  (org-depend-tag-blocked nil)
-	  org-blocked-by-checkboxes)
+    (let ((inhibit-read-only t))
       (goto-char (point-min))
       (while (let ((pos (text-property-not-all
-			 (point) (point-max) 'todo-state nil)))
+			 (point) (point-max) 'org-todo-blocked nil)))
 	       (when pos (goto-char pos)))
-	(setq org-blocked-by-checkboxes nil)
-	(let ((marker (org-get-at-bol 'org-hd-marker)))
-	  (when (and (markerp marker)
-		     (with-current-buffer (marker-buffer marker)
-		       (save-excursion (goto-char marker)
-				       (org-entry-blocked-p))))
-	    ;; Entries blocked by checkboxes cannot be made invisible.
-	    ;; See `org-agenda-dim-blocked-tasks' for details.
-	    (let* ((really-invisible
-		    (and (not org-blocked-by-checkboxes)
-			 (or invisible (eq org-agenda-dim-blocked-tasks
-					   'invisible))))
-		   (ov (make-overlay (if really-invisible (line-end-position 0)
-				       (line-beginning-position))
-				     (line-end-position))))
-	      (if really-invisible (overlay-put ov 'invisible t)
-		(overlay-put ov 'face 'org-agenda-dimmed-todo-face))
-	      (overlay-put ov 'org-type 'org-blocked-todo))))
+	(let* ((invisible (eq (org-get-at-bol 'org-todo-blocked) 'invisible))
+	       (ov (make-overlay (if invisible
+				     (line-end-position 0)
+				   (line-beginning-position))
+				 (line-end-position))))
+	  (if invisible
+	      (overlay-put ov 'invisible t)
+	    (overlay-put ov 'face 'org-agenda-dimmed-todo-face))
+	  (overlay-put ov 'org-type 'org-blocked-todo))
 	(forward-line))))
   (when (called-interactively-p 'interactive)
     (message "Dim or hide blocked tasks...done")))
 
+(defun org-agenda--mark-blocked-entry (entry)
+  "For ENTRY a string with the text property `org-hd-marker', if
+the header at `org-hd-marker' is blocked according to
+`org-entry-blocked-p', then if `org-agenda-dim-blocked-tasks' is
+'invisible and the header is not blocked by checkboxes, set the
+text property `org-todo-blocked' to 'invisible, otherwise set it
+to t."
+  (when (get-text-property 0 'todo-state entry)
+    (let ((entry-marker (get-text-property 0 'org-hd-marker entry))
+          (org-blocked-by-checkboxes nil)
+	  ;; Necessary so that `org-entry-blocked-p' does not change the buffer.
+          (org-depend-tag-blocked nil))
+      (when entry-marker
+	(let ((blocked
+	       (with-current-buffer (marker-buffer entry-marker)
+		 (save-mark-and-excursion
+		  (goto-char entry-marker)
+		  (org-entry-blocked-p)))))
+	  (when blocked
+	    (let ((really-invisible
+		   (and (not org-blocked-by-checkboxes)
+			(eq org-agenda-dim-blocked-tasks 'invisible))))
+	      (put-text-property
+	       0 (length entry) 'org-todo-blocked
+	       (if really-invisible 'invisible t)
+	       entry))))))
+    entry))
+
 (defvar org-agenda-skip-function nil
   "Function to be called at each match during agenda construction.
 If this function returns nil, the current match should not be skipped.
@@ -6781,6 +6798,8 @@ The optional argument TYPE tells the agenda type."
       (setq list (org-agenda-limit-entries list 'tags max-tags)))
     (when max-entries
       (setq list (org-agenda-limit-entries list 'org-hd-marker max-entries)))
+    (when (and org-agenda-dim-blocked-tasks org-blocker-hook)
+      (setq list (mapcar #'org-agenda--mark-blocked-entry list)))
     (mapconcat 'identity list "\n")))
 
 (defun org-agenda-limit-entries (list prop limit &optional fn)
-- 
2.9.2.windows.1

Re: [Patch] Support for dimming local to each agenda

[Nicolas Goaziou]

Hello,

Yuri Lensky <ydl@ydl.cm> writes:

> Thanks. I have attached an updated patch that addresses your points (with
> some additional cleanup).

Thank you.

>> Why is `save-mark-and-excursion' needed?
>>
> Because we are moving point to get to the heading, and in principle hooks
> in org-blocker-hook can change point/mark.

Since we don't care about mark, isn't `save-excursion' sufficient?

Regards,

-- 
Nicolas Goaziou

Re: [Patch] Support for dimming local to each agenda

[Yuri Lensky]

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

Why not save mark? This allows user-defined hooks in org-blocker-hooks more
freedom. That said, I am not sure of org-mode project conventions, so if
`save-excursion' is preferred I can change it to that.

Thanks.

On Fri, Jul 14, 2017 at 1:29 AM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Yuri Lensky <ydl@ydl.cm> writes:
>
> > Thanks. I have attached an updated patch that addresses your points (with
> > some additional cleanup).
>
> Thank you.
>
> >> Why is `save-mark-and-excursion' needed?
> >>
> > Because we are moving point to get to the heading, and in principle hooks
> > in org-blocker-hook can change point/mark.
>
> Since we don't care about mark, isn't `save-excursion' sufficient?
>
> Regards,
>
> --
> Nicolas Goaziou
>

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

Re: [Patch] Support for dimming local to each agenda

[Nicolas Goaziou]

Yuri Lensky <ydl@ydl.cm> writes:

> Why not save mark? 

We usually do not mess with it, unless this is explicit in the function
documentation, e.g., `org-mark-ring-push'.

> This allows user-defined hooks in org-blocker-hooks more freedom.

I'm not sure this is a sufficient reason, really. Besides it's
debatable.

> That said, I am not sure of org-mode project conventions, so if
> `save-excursion' is preferred I can change it to that.

I'm going to do the change myself and apply you patch. No need for
another iteration.

Thank you!

Regards,

Re: [Patch] Support for dimming local to each agenda

[Nicolas Goaziou]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Yuri Lensky <ydl@ydl.cm> writes:
>
>> Why not save mark? 
>
> We usually do not mess with it, unless this is explicit in the function
> documentation, e.g., `org-mark-ring-push'.
>
>> This allows user-defined hooks in org-blocker-hooks more freedom.
>
> I'm not sure this is a sufficient reason, really. Besides it's
> debatable.
>
>> That said, I am not sure of org-mode project conventions, so if
>> `save-excursion' is preferred I can change it to that.
>
> I'm going to do the change myself and apply you patch. No need for
> another iteration.

Done. Would you mind providing an ORG-NEWS entry?

Thank you!