Hello, Allen Li <darkfel...@felesatra.moe> 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
