[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] Bug: org-agenda-overriding-columns-format destroyed on revert [9
From: |
Nicolas Goaziou |
Subject: |
Re: [O] Bug: org-agenda-overriding-columns-format destroyed on revert [9.2.1 (9.2.1-2-gc6d37c-elpaplus)] |
Date: |
Mon, 18 Feb 2019 22:16:21 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Hello,
Allen Li <address@hidden> writes:
> I have attached a patch implementing this on maint.
Thank you. Some comments follow.
> Subject: [PATCH] Fix buffer local org-agenda-overriding-columns-format bug
>
> Setting org-agenda-overriding-columns-format as a buffer local value
> interferes with how it is used as a dynamically scoped var, so use a
> separate variable for buffer local setting.
You need to include variables and functions modified in the commit
message, so grepping through them more fruitful.
> (defvar org-agenda-overriding-columns-format) ; From org-colview.el
> +(defvar org-agenda-local-overriding-columns-format) ; From org-colview.el
You can remove the inline comment at the end of the line (and the one
above).
> (defun org-agenda-finalize ()
> "Finishing touch for the agenda buffer, called just before displaying it."
> (unless org-agenda-multi
> @@ -3783,7 +3784,7 @@ FILTER-ALIST is an alist of filters we need to apply
> when
> (unless org-agenda-with-colors
> (remove-text-properties (point-min) (point-max) '(face nil)))
> (when (bound-and-true-p org-agenda-overriding-columns-format)
> - (setq-local org-agenda-overriding-columns-format
> + (setq-local org-agenda-local-overriding-columns-format
Since this is a local variable, `setq' is enough, isn't it?
> org-agenda-overriding-columns-format))
> (when org-agenda-view-columns-initially
> (org-agenda-columns))
> diff --git a/lisp/org-colview.el b/lisp/org-colview.el
> index 746426bc7..2fbb5aa6c 100644
> --- a/lisp/org-colview.el
> +++ b/lisp/org-colview.el
> @@ -567,7 +567,13 @@ for the duration of the command.")
>
> (defvar org-agenda-overriding-columns-format nil
> "When set, overrides any other format definition for the agenda.
> -Don't set this, this is meant for dynamic scoping.")
> +Don't set this, this is meant for dynamic scoping. Set
> +`org-agenda-local-overriding-columns-format' instead.")
The first line of a docstring cannot contain an incomplete sentence (due
to `apropos` design).
> +(defvar-local org-agenda-local-overriding-columns-format nil
> + "When set, overrides any other format definition for the agenda.
> +This can be set as a buffer local value to avoid interfering with
> +dynamic scoping for `org-agenda-overriding-columns-format'.")
The two variable names are somewhat confusing. Could
`org-agenda-local-overriding-columns-format' be renamed into
`org-local-columns-format'? Since it is defined in "org-colview.el", it
doesn't deserve the "org-agenda" prefix (and neither does the original
`org-agenda-overriding-columns-format'.).
Regards,
--
Nicolas Goaziou