From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuri Lensky Subject: Re: [Patch] Support for dimming local to each agenda Date: Thu, 13 Jul 2017 11:09:39 -0700 Message-ID: References: <8760ewfqja.fsf@nicolasgoaziou.fr> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="94eb2c1891c40e7041055436d657" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:43554) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dViYb-0007QO-3V for emacs-orgmode@gnu.org; Thu, 13 Jul 2017 14:09:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dViYZ-00036V-ME for emacs-orgmode@gnu.org; Thu, 13 Jul 2017 14:09:45 -0400 Received: from mail-oi0-x236.google.com ([2607:f8b0:4003:c06::236]:36334) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dViYZ-00030B-ER for emacs-orgmode@gnu.org; Thu, 13 Jul 2017 14:09:43 -0400 Received: by mail-oi0-x236.google.com with SMTP id x187so52884067oig.3 for ; Thu, 13 Jul 2017 11:09:40 -0700 (PDT) In-Reply-To: <8760ewfqja.fsf@nicolasgoaziou.fr> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Nicolas Goaziou Cc: "emacs-orgmode@gnu.org" , Yuri Lensky --94eb2c1891c40e7041055436d657 Content-Type: multipart/alternative; boundary="94eb2c1891c40e7039055436d655" --94eb2c1891c40e7039055436d655 Content-Type: text/plain; charset="UTF-8" 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 wrote: > Hello, > > Yuri Lensky 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" > > 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 --94eb2c1891c40e7039055436d655 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks. I have attached an updated patch that addresses yo= ur points (with some additional cleanup).
On Thu, Jul 13, 2017 at 5:51 AM, Nicolas Goazio= u <mail@nicolasgoaziou.fr> wrote:
Hello,

Yuri Lensky <ydl@ydl.cm> writes:
> Composite agenda views can now set org-agenda-dim-blocked-tasks locall= y.
> From 52f8bf79a198fa2e7f131c2a015a7c9400a403ac Mon Sep 17 0= 0: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<= br> > 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.=C2=A0

> +(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.=C2= =A0

> +=C2=A0 "Mark a blocked entry in text properties."

The docstring should specifiy what ENTRY is.
Fixed.=C2= =A0

> +=C2=A0 (when (get-text-property 0 'todo-state entry)
> +=C2=A0 =C2=A0 (let ((entry-marker (get-text-property 0 'org-hd-ma= rker entry))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (org-blocked-by-checkboxes nil) > +=C2=A0 =C2=A0 =C2=A0 =C2=A0;; Necessary so that org-entry-blocked-p d= oes not change the buffer

`org-entry-blocked-p'

Missing full stop, too.
Fixed.=C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (org-depend-tag-blocked nil))
> +=C2=A0 =C2=A0 =C2=A0 (let ((blocked
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (with-current-buffer (marker-buffe= r entry-marker)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (save-mark-and-excursion
Why is `save-mark-and-excursion' needed?
Because w= e are moving point to get to the heading, and in principle hooks in org-blo= cker-hook can change point/mark.=C2=A0

> +=C2=A0 =C2=A0 (when (and org-agenda-dim-blocked-tasks org-blocker-hoo= k)
> +=C2=A0 =C2=A0 =C2=A0 (setq list (mapcar 'org-agenda-mark-blocked-= entry list)))

Nitpick: (mapcar #'org-agenda-mark-blocked-entry list)
Fixed.=C2=A0

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 feat= ure it is easy to see if it works.

Regards,

--
Nicolas Goaziou
--94eb2c1891c40e7039055436d655-- --94eb2c1891c40e7041055436d657 Content-Type: application/octet-stream; name="0001-org-agenda.el-Support-for-dimming-local-to-each-agen.patch" Content-Disposition: attachment; filename="0001-org-agenda.el-Support-for-dimming-local-to-each-agen.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_j52r0oyq0 RnJvbSAwZDk2ZGM4MjcwMzFiOWU2NTk4ODk3YmM1YmNlNjIxNmNiYTU2ZTdlIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiAiWXVyaSBELiBMZW5za3kiIDx5ZGxlbnNreUBnbWFpbC5jb20+ CkRhdGU6IE1vbiwgMTAgSnVsIDIwMTcgMTk6MjE6MzkgLTA3MDAKU3ViamVjdDogW1BBVENIXSBv cmctYWdlbmRhLmVsOiBTdXBwb3J0IGZvciBkaW1taW5nIGxvY2FsIHRvIGVhY2ggYWdlbmRhLgoK Q29tcG9zaXRlIGFnZW5kYSB2aWV3cyBjb3VsZCBub3Qgc2VwYXJhdGVseSBzcGVjaWZ5IHdoZXRo ZXIgdG8gZGltCmJsb2NrZWQgdGFza3MuCgoqIGxpc3Avb3JnLWFnZW5kYS5lbCAob3JnLWFnZW5k YS0tbWFyay1ibG9ja2VkLWVudHJ5KTogTmV3IGZ1bmN0aW9uCiAgdGhhdCBtYXJrcyB3aGV0aGVy IGFuIGVudHJ5IChhIHN0cmluZyB3aXRoIHRleHQgcHJvcGVydGllcyBhcyBwYXNzZWQKICBhcm91 bmQgaW4gYWdlbmRhIGZ1bmN0aW9ucykgaXMgYmxvY2tlZCBhY2NvcmRpbmcgdG8KICBvcmctZW50 cnktYmxvY2tlZC1wLgoKICAob3JnLWFnZW5kYS1kaW0tYmxvY2tlZC10YXNrcyk6IE1vZGlmaWVk IHRvIHdvcmsgd2l0aCB0ZXh0CiAgcHJvcGVydGllcyBzZXQgYnkgb3JnLWFnZW5kYS0tbWFya2Vk LWJsb2NrZWQtZW50cnkuCi0tLQogbGlzcC9vcmctYWdlbmRhLmVsIHwgNjMgKysrKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKystLS0tLS0tLS0tLS0tLS0tLS0tCiAxIGZpbGUgY2hhbmdl ZCwgNDEgaW5zZXJ0aW9ucygrKSwgMjIgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvbGlzcC9v cmctYWdlbmRhLmVsIGIvbGlzcC9vcmctYWdlbmRhLmVsCmluZGV4IDlhYzRmNjUuLjVlYWJkNTcg MTAwNjQ0Ci0tLSBhL2xpc3Avb3JnLWFnZW5kYS5lbAorKysgYi9saXNwL29yZy1hZ2VuZGEuZWwK QEAgLTM4ODYsMzUgKzM4ODYsNTIgQEAgZGltbWluZyB0aGVtLiIKICAgICAod2hlbiAoZXEgKG92 ZXJsYXktZ2V0IG8gJ29yZy10eXBlKSAnb3JnLWJsb2NrZWQtdG9kbykKICAgICAgIChkZWxldGUt b3ZlcmxheSBvKSkpCiAgIChzYXZlLWV4Y3Vyc2lvbgotICAgIChsZXQgKChpbmhpYml0LXJlYWQt b25seSB0KQotCSAgKG9yZy1kZXBlbmQtdGFnLWJsb2NrZWQgbmlsKQotCSAgb3JnLWJsb2NrZWQt YnktY2hlY2tib3hlcykKKyAgICAobGV0ICgoaW5oaWJpdC1yZWFkLW9ubHkgdCkpCiAgICAgICAo Z290by1jaGFyIChwb2ludC1taW4pKQogICAgICAgKHdoaWxlIChsZXQgKChwb3MgKHRleHQtcHJv cGVydHktbm90LWFsbAotCQkJIChwb2ludCkgKHBvaW50LW1heCkgJ3RvZG8tc3RhdGUgbmlsKSkp CisJCQkgKHBvaW50KSAocG9pbnQtbWF4KSAnb3JnLXRvZG8tYmxvY2tlZCBuaWwpKSkKIAkgICAg ICAgKHdoZW4gcG9zIChnb3RvLWNoYXIgcG9zKSkpCi0JKHNldHEgb3JnLWJsb2NrZWQtYnktY2hl Y2tib3hlcyBuaWwpCi0JKGxldCAoKG1hcmtlciAob3JnLWdldC1hdC1ib2wgJ29yZy1oZC1tYXJr ZXIpKSkKLQkgICh3aGVuIChhbmQgKG1hcmtlcnAgbWFya2VyKQotCQkgICAgICh3aXRoLWN1cnJl bnQtYnVmZmVyIChtYXJrZXItYnVmZmVyIG1hcmtlcikKLQkJICAgICAgIChzYXZlLWV4Y3Vyc2lv biAoZ290by1jaGFyIG1hcmtlcikKLQkJCQkgICAgICAgKG9yZy1lbnRyeS1ibG9ja2VkLXApKSkp Ci0JICAgIDs7IEVudHJpZXMgYmxvY2tlZCBieSBjaGVja2JveGVzIGNhbm5vdCBiZSBtYWRlIGlu dmlzaWJsZS4KLQkgICAgOzsgU2VlIGBvcmctYWdlbmRhLWRpbS1ibG9ja2VkLXRhc2tzJyBmb3Ig ZGV0YWlscy4KLQkgICAgKGxldCogKChyZWFsbHktaW52aXNpYmxlCi0JCSAgICAoYW5kIChub3Qg b3JnLWJsb2NrZWQtYnktY2hlY2tib3hlcykKLQkJCSAob3IgaW52aXNpYmxlIChlcSBvcmctYWdl bmRhLWRpbS1ibG9ja2VkLXRhc2tzCi0JCQkJCSAgICdpbnZpc2libGUpKSkpCi0JCSAgIChvdiAo bWFrZS1vdmVybGF5IChpZiByZWFsbHktaW52aXNpYmxlIChsaW5lLWVuZC1wb3NpdGlvbiAwKQot CQkJCSAgICAgICAobGluZS1iZWdpbm5pbmctcG9zaXRpb24pKQotCQkJCSAgICAgKGxpbmUtZW5k LXBvc2l0aW9uKSkpKQotCSAgICAgIChpZiByZWFsbHktaW52aXNpYmxlIChvdmVybGF5LXB1dCBv diAnaW52aXNpYmxlIHQpCi0JCShvdmVybGF5LXB1dCBvdiAnZmFjZSAnb3JnLWFnZW5kYS1kaW1t ZWQtdG9kby1mYWNlKSkKLQkgICAgICAob3ZlcmxheS1wdXQgb3YgJ29yZy10eXBlICdvcmctYmxv Y2tlZC10b2RvKSkpKQorCShsZXQqICgoaW52aXNpYmxlIChlcSAob3JnLWdldC1hdC1ib2wgJ29y Zy10b2RvLWJsb2NrZWQpICdpbnZpc2libGUpKQorCSAgICAgICAob3YgKG1ha2Utb3ZlcmxheSAo aWYgaW52aXNpYmxlCisJCQkJICAgICAobGluZS1lbmQtcG9zaXRpb24gMCkKKwkJCQkgICAobGlu ZS1iZWdpbm5pbmctcG9zaXRpb24pKQorCQkJCSAobGluZS1lbmQtcG9zaXRpb24pKSkpCisJICAo aWYgaW52aXNpYmxlCisJICAgICAgKG92ZXJsYXktcHV0IG92ICdpbnZpc2libGUgdCkKKwkgICAg KG92ZXJsYXktcHV0IG92ICdmYWNlICdvcmctYWdlbmRhLWRpbW1lZC10b2RvLWZhY2UpKQorCSAg KG92ZXJsYXktcHV0IG92ICdvcmctdHlwZSAnb3JnLWJsb2NrZWQtdG9kbykpCiAJKGZvcndhcmQt bGluZSkpKSkKICAgKHdoZW4gKGNhbGxlZC1pbnRlcmFjdGl2ZWx5LXAgJ2ludGVyYWN0aXZlKQog ICAgIChtZXNzYWdlICJEaW0gb3IgaGlkZSBibG9ja2VkIHRhc2tzLi4uZG9uZSIpKSkKIAorKGRl ZnVuIG9yZy1hZ2VuZGEtLW1hcmstYmxvY2tlZC1lbnRyeSAoZW50cnkpCisgICJGb3IgRU5UUlkg YSBzdHJpbmcgd2l0aCB0aGUgdGV4dCBwcm9wZXJ0eSBgb3JnLWhkLW1hcmtlcicsIGlmCit0aGUg aGVhZGVyIGF0IGBvcmctaGQtbWFya2VyJyBpcyBibG9ja2VkIGFjY29yZGluZyB0bworYG9yZy1l bnRyeS1ibG9ja2VkLXAnLCB0aGVuIGlmIGBvcmctYWdlbmRhLWRpbS1ibG9ja2VkLXRhc2tzJyBp cworJ2ludmlzaWJsZSBhbmQgdGhlIGhlYWRlciBpcyBub3QgYmxvY2tlZCBieSBjaGVja2JveGVz LCBzZXQgdGhlCit0ZXh0IHByb3BlcnR5IGBvcmctdG9kby1ibG9ja2VkJyB0byAnaW52aXNpYmxl LCBvdGhlcndpc2Ugc2V0IGl0Cit0byB0LiIKKyAgKHdoZW4gKGdldC10ZXh0LXByb3BlcnR5IDAg J3RvZG8tc3RhdGUgZW50cnkpCisgICAgKGxldCAoKGVudHJ5LW1hcmtlciAoZ2V0LXRleHQtcHJv cGVydHkgMCAnb3JnLWhkLW1hcmtlciBlbnRyeSkpCisgICAgICAgICAgKG9yZy1ibG9ja2VkLWJ5 LWNoZWNrYm94ZXMgbmlsKQorCSAgOzsgTmVjZXNzYXJ5IHNvIHRoYXQgYG9yZy1lbnRyeS1ibG9j a2VkLXAnIGRvZXMgbm90IGNoYW5nZSB0aGUgYnVmZmVyLgorICAgICAgICAgIChvcmctZGVwZW5k LXRhZy1ibG9ja2VkIG5pbCkpCisgICAgICAod2hlbiBlbnRyeS1tYXJrZXIKKwkobGV0ICgoYmxv Y2tlZAorCSAgICAgICAod2l0aC1jdXJyZW50LWJ1ZmZlciAobWFya2VyLWJ1ZmZlciBlbnRyeS1t YXJrZXIpCisJCSAoc2F2ZS1tYXJrLWFuZC1leGN1cnNpb24KKwkJICAoZ290by1jaGFyIGVudHJ5 LW1hcmtlcikKKwkJICAob3JnLWVudHJ5LWJsb2NrZWQtcCkpKSkpCisJICAod2hlbiBibG9ja2Vk CisJICAgIChsZXQgKChyZWFsbHktaW52aXNpYmxlCisJCSAgIChhbmQgKG5vdCBvcmctYmxvY2tl ZC1ieS1jaGVja2JveGVzKQorCQkJKGVxIG9yZy1hZ2VuZGEtZGltLWJsb2NrZWQtdGFza3MgJ2lu dmlzaWJsZSkpKSkKKwkgICAgICAocHV0LXRleHQtcHJvcGVydHkKKwkgICAgICAgMCAobGVuZ3Ro IGVudHJ5KSAnb3JnLXRvZG8tYmxvY2tlZAorCSAgICAgICAoaWYgcmVhbGx5LWludmlzaWJsZSAn aW52aXNpYmxlIHQpCisJICAgICAgIGVudHJ5KSkpKSkpCisgICAgZW50cnkpKQorCiAoZGVmdmFy IG9yZy1hZ2VuZGEtc2tpcC1mdW5jdGlvbiBuaWwKICAgIkZ1bmN0aW9uIHRvIGJlIGNhbGxlZCBh dCBlYWNoIG1hdGNoIGR1cmluZyBhZ2VuZGEgY29uc3RydWN0aW9uLgogSWYgdGhpcyBmdW5jdGlv biByZXR1cm5zIG5pbCwgdGhlIGN1cnJlbnQgbWF0Y2ggc2hvdWxkIG5vdCBiZSBza2lwcGVkLgpA QCAtNjc4MSw2ICs2Nzk4LDggQEAgVGhlIG9wdGlvbmFsIGFyZ3VtZW50IFRZUEUgdGVsbHMgdGhl IGFnZW5kYSB0eXBlLiIKICAgICAgIChzZXRxIGxpc3QgKG9yZy1hZ2VuZGEtbGltaXQtZW50cmll cyBsaXN0ICd0YWdzIG1heC10YWdzKSkpCiAgICAgKHdoZW4gbWF4LWVudHJpZXMKICAgICAgIChz ZXRxIGxpc3QgKG9yZy1hZ2VuZGEtbGltaXQtZW50cmllcyBsaXN0ICdvcmctaGQtbWFya2VyIG1h eC1lbnRyaWVzKSkpCisgICAgKHdoZW4gKGFuZCBvcmctYWdlbmRhLWRpbS1ibG9ja2VkLXRhc2tz IG9yZy1ibG9ja2VyLWhvb2spCisgICAgICAoc2V0cSBsaXN0IChtYXBjYXIgIydvcmctYWdlbmRh LS1tYXJrLWJsb2NrZWQtZW50cnkgbGlzdCkpKQogICAgIChtYXBjb25jYXQgJ2lkZW50aXR5IGxp c3QgIlxuIikpKQogCiAoZGVmdW4gb3JnLWFnZW5kYS1saW1pdC1lbnRyaWVzIChsaXN0IHByb3Ag bGltaXQgJm9wdGlvbmFsIGZuKQotLSAKMi45LjIud2luZG93cy4xCgo= --94eb2c1891c40e7041055436d657--