From: Yuri Lensky <ydl@ydl.cm>
To: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Cc: "emacs-orgmode@gnu.org" <emacs-orgmode@gnu.org>,
Yuri Lensky <ydl@ydl.cm>
Subject: Re: [Patch] Support for dimming local to each agenda
Date: Thu, 13 Jul 2017 11:09:39 -0700 [thread overview]
Message-ID: <CAC38U-dTU0X-Ka9fvxPH+9DDaTv=jDLmZrZGLXpx+k=Kq3Bc=w@mail.gmail.com> (raw)
In-Reply-To: <8760ewfqja.fsf@nicolasgoaziou.fr>
[-- 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
next prev parent reply other threads:[~2017-07-13 18:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-12 0:39 [Patch] Support for dimming local to each agenda Yuri Lensky
2017-07-13 12:51 ` Nicolas Goaziou
2017-07-13 18:09 ` Yuri Lensky [this message]
2017-07-14 8:29 ` Nicolas Goaziou
2017-07-14 9:22 ` Yuri Lensky
2017-07-14 9:39 ` Nicolas Goaziou
2017-07-14 12:18 ` Nicolas Goaziou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAC38U-dTU0X-Ka9fvxPH+9DDaTv=jDLmZrZGLXpx+k=Kq3Bc=w@mail.gmail.com' \
--to=ydl@ydl.cm \
--cc=emacs-orgmode@gnu.org \
--cc=mail@nicolasgoaziou.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).