Re: Using lexical-binding
Kyle, > Hmm, given that the lexical-binding change to ob-core was back in Org > 9.0 (November 2016), it seems like dynamic scoping wasn't really being > relied on (or, if it was, downstream code has already been adjusted). > In my view it'd be better to stick with lexical scoping for these > variables, with callers explicitly passing the subset of needed > variables to the underlying function(s). yes, that makes sense. thanks. cheers, Greg
Re: Using lexical-binding
Greg Minshall writes: > Kyle, > > thanks. i see. i wondered why the talk was all about agendas. > > since, in my (brand new, experimenting) use of > =org-babel-map-src-blocks=, i'm calling a function, and that function is > trying to de-reference, e.g., =beg-block=, i get an error. Thanks for the details. > it is (or does seem to be) the case that if the macro included all the > valueless =defvars=, a function called from it has access to all those. > i don't know if this would be a useful modification. Hmm, given that the lexical-binding change to ob-core was back in Org 9.0 (November 2016), it seems like dynamic scoping wasn't really being relied on (or, if it was, downstream code has already been adjusted). In my view it'd be better to stick with lexical scoping for these variables, with callers explicitly passing the subset of needed variables to the underlying function(s).
Re: Using lexical-binding
Kyle, thanks. i see. i wondered why the talk was all about agendas. since, in my (brand new, experimenting) use of =org-babel-map-src-blocks=, i'm calling a function, and that function is trying to de-reference, e.g., =beg-block=, i get an error. it is (or does seem to be) the case that if the macro included all the valueless =defvars=, a function called from it has access to all those. i don't know if this would be a useful modification. cheers, Greg
Re: Using lexical-binding
Greg Minshall writes: > hi. i just upgraded to > : Org mode version 9.4.4 (9.4.4-27-gb712b9-elpa @ > /home/minshall/.emacs.d/elpa/org-20210315/) > > and, have also just started playing with > (org-babel-map-inline-src-blocks), the documentation for which says > > During evaluation of BODY the following local variables > are set relative to the currently matched code block. > ... > Is there a specific error/misbehavior that you're seeing? The patch in this thread switched only one file, lisp/org-agenda.el, over to lexical binding. org-babel-map-inline-src-blocks is in ob-core, and that file has used lexical binding since 6cefae163 (ob-core: Use lexical binding, 2016-06-20). > but, iiuc, that relies on dynamic binding. so, as =lexical-binding= is > =t=, i don't have access to those appealing variables. org-babel-map-inline-src-blocks is a macro, and these variables are defined in its expansion. Try: (pp-macroexpand-expression '(org-babel-map-src-blocks nil (message "%d %s %s" beg-block lang body)))
Re: Using lexical-binding
> but, iiuc, that relies on dynamic binding. so, as =lexical-binding= is > =t=, i don't have access to those appealing variables. from reading the elisp manual, it seems that one could define those variables to be "special variables", and, iiuc, one can achieve this by using a =defvar= without a value, previous to the =let= where values are assigned. something like (for just full-block): diff --git a/lisp/ob-core.el b/lisp/ob-core.el index af2c9912e..a0528bb06 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -1121,6 +1121,7 @@ end-body - point at the end of the body" (while (re-search-forward org-babel-src-block-regexp nil t) (when (org-babel-active-location-p) (goto-char (match-beginning 0)) +(defvar full-block) (let ((full-block (match-string 0)) (beg-block (match-beginning 0)) (end-block (match-end 0)) i could do a patch in this style, for all these variables. cheers, Greg
Re: Using lexical-binding
hi. i just upgraded to : Org mode version 9.4.4 (9.4.4-27-gb712b9-elpa @ /home/minshall/.emacs.d/elpa/org-20210315/) and, have also just started playing with (org-babel-map-inline-src-blocks), the documentation for which says During evaluation of BODY the following local variables are set relative to the currently matched code block. ... but, iiuc, that relies on dynamic binding. so, as =lexical-binding= is =t=, i don't have access to those appealing variables. am i missing something? or, is this a place where the "API" is no longer compatible? should those variables somehow be passed as a parameter (alist?) to =,@body=? or, (let ((lexical-binding nil)) ...)? (if that would work.) cheers, Greg
Re: Using lexical-binding
Thanks. So now, I'm just waiting for that code to make its way to Emacs's `master` branch (which I guess first means to make its way to an Org release, so I had better find something else to do in the mean time). Stefan
Re: Using lexical-binding
Stefan Monnier writes: > Any chance you could put this in the `Package-Requires:`? Sure, added (5263eff5a). > Note that all the uses I introduced of `with-suppressed-warnings` only > wrap a very small amount of code, so you could also replace them with > `with-no-warnings` (added back in Emacs-22). Good point. I've switched to using with-suppressed-warnings and applied the patch to master (129c33ddd). Thanks again.
Re: Using lexical-binding
Hi Kyle,
>> Subject: [PATCH] * lisp/org-agenda.el: Use lexical-binding
> [...]
>> +(pcase type
>> + ('agenda
>> + (org-agenda-list current-prefix-arg))
>
> Unfortunately Org's minimum Emacs version is still Emacs 24.3.
Sorry 'bout that. I keep forgetting about this detail of `pcase` past.
Any chance you could put this in the `Package-Requires:`?
>> + (let* ((gprops (nth 1 series))
>> + (gvars (mapcar #'car gprops))
>> + (gvals (mapcar (lambda (binding) (eval (cadr binding) t)) gprops)))
>> +(cl-progv gvars gvals (org-agenda-prepare name))
>> +;; We need to reset agenda markers here, because when constructing a
>> +;; block agenda, the individual blocks do not do that.
>> +(org-agenda-reset-markers)
>> +(with-suppressed-warnings ((lexical match))
>
> ... I believe with-suppressed-warnings isn't available until Emacs 27.1,
> right?
Ooh, right, and that is not just a little detail, I very much know
about that. I don't have any excuse for this one (it's just
a careless copy).
> Any objections to me squashing the below changes into your patch?
Of course not.
> +(if (fboundp 'with-suppressed-warnings) ; Introduced in Emacs 27.1.
> +(defalias 'org-with-suppressed-warnings 'with-suppressed-warnings)
> + (defmacro org-with-suppressed-warnings (_warnings body)
> +(declare (debug (sexp body)) (indent 1))
> +`(progn ,@body)))
Note that all the uses I introduced of `with-suppressed-warnings` only
wrap a very small amount of code, so you could also replace them with
`with-no-warnings` (added back in Emacs-22).
Stefan
Re: Using lexical-binding
Looking at this one more time before applying, I noticed a couple of
backward compatibility issues.
Stefan Monnier writes:
> Subject: [PATCH] * lisp/org-agenda.el: Use lexical-binding
[...]
> + (pcase type
> + ('agenda
> +(org-agenda-list current-prefix-arg))
Unfortunately Org's minimum Emacs version is still Emacs 24.3. I'd like
to drop Emacs 24 support soon, but that hasn't been discussed or
announced.
And...
> + (let* ((gprops (nth 1 series))
> + (gvars (mapcar #'car gprops))
> + (gvals (mapcar (lambda (binding) (eval (cadr binding) t)) gprops)))
> +(cl-progv gvars gvals (org-agenda-prepare name))
> +;; We need to reset agenda markers here, because when constructing a
> +;; block agenda, the individual blocks do not do that.
> +(org-agenda-reset-markers)
> +(with-suppressed-warnings ((lexical match))
... I believe with-suppressed-warnings isn't available until Emacs 27.1,
right?
Any objections to me squashing the below changes into your patch?
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 001ca4b1b..d08cab061 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -2938,30 +2938,30 @@ (defun org-agenda ( arg keys restriction)
(mapcar #'car lprops)
(mapcar (lambda (binding) (eval (cadr binding) t)) lprops)
(pcase type
- ('agenda
+ (`agenda
(org-agenda-list current-prefix-arg))
- ('agenda*
+ (`agenda*
(org-agenda-list current-prefix-arg nil nil t))
- ('alltodo
+ (`alltodo
(org-todo-list current-prefix-arg))
- ('search
+ (`search
(org-search-view current-prefix-arg org-match nil))
- ('stuck
+ (`stuck
(org-agenda-list-stuck-projects current-prefix-arg))
- ('tags
+ (`tags
(org-tags-view current-prefix-arg org-match))
- ('tags-todo
+ (`tags-todo
(org-tags-view '(4) org-match))
- ('todo
+ (`todo
(org-todo-list org-match))
- ('tags-tree
+ (`tags-tree
(org-check-for-org-mode)
(org-match-sparse-tree current-prefix-arg org-match))
- ('todo-tree
+ (`todo-tree
(org-check-for-org-mode)
(org-occur (concat "^" org-outline-regexp "[ \t]*"
(regexp-quote org-match) "\\>")))
- ('occur-tree
+ (`occur-tree
(org-check-for-org-mode)
(org-occur org-match))
((pred functionp)
@@ -3263,7 +3263,7 @@ (defun org-agenda-run-series (name series)
;; We need to reset agenda markers here, because when constructing a
;; block agenda, the individual blocks do not do that.
(org-agenda-reset-markers)
-(with-suppressed-warnings ((lexical match))
+(org-with-suppressed-warnings ((lexical match))
(defvar match)) ;Used via the `eval' below.
(let* ((org-agenda-multi t)
;; FIXME: Redo should contain lists of (FUNS . ARGS) rather
@@ -3285,21 +3285,21 @@ (defun org-agenda-run-series (name series)
(lvals (mapcar (lambda (binding) (eval (cadr binding) t))
lprops)))
(cl-progv (append gvars lvars) (append gvals lvals)
(pcase type
- ('agenda
+ (`agenda
(call-interactively 'org-agenda-list))
- ('agenda*
+ (`agenda*
(funcall 'org-agenda-list nil nil t))
- ('alltodo
+ (`alltodo
(call-interactively 'org-todo-list))
- ('search
+ (`search
(org-search-view current-prefix-arg match nil))
- ('stuck
+ (`stuck
(call-interactively 'org-agenda-list-stuck-projects))
- ('tags
+ (`tags
(org-tags-view current-prefix-arg match))
- ('tags-todo
+ (`tags-todo
(org-tags-view '(4) match))
- ('todo
+ (`todo
(org-todo-list match))
((pred fboundp)
(funcall type match))
@@ -5363,7 +5363,7 @@ (defun org-diary ( args)
The function expects the lisp variables `entry' and `date' to be provided
by the caller, because this is how the calendar works. Don't use this
function from a program - use `org-agenda-get-day-entries' instead."
- (with-suppressed-warnings ((lexical date entry)) (defvar date) (defvar
entry))
+ (org-with-suppressed-warnings ((lexical date entry)) (defvar date) (defvar
entry))
(when (> (- (float-time)
Re: Using lexical-binding
>> Should I send a rebased patch for inclusion > Yes, please. Here it is, Stefan >From ba61c9660fc09321f9dfe5f746705f5d1202c474 Mon Sep 17 00:00:00 2001 From: Stefan Monnier Date: Tue, 23 Feb 2021 15:47:29 -0500 Subject: [PATCH] * lisp/org-agenda.el: Use lexical-binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Removed the global (defvar date) and (defvar entry) so as not to conflict with function arguments of that name. Instead I added such `defvar`s in the body of each of the functions where it seemed needed. - I added some FIXMEs for some issues I found along the way. - Added an `org-dlet` macro, just like I had done for `calendar-dlet`, but I also use `defvar` "manually" at some places, when splitting an existing `let` into a mix of `let`s and `dlet`s seemed too much trouble. - Removed uses of `org-let and `org-let2` not only because I consider them offensive to my sense of aesthetics but also because they're basically incompatible with lexical scoping. I replaced them with uses of `cl-progv` which are a bit more verbose. Maybe we should define some `org-progv` macro on top of `cl-progv` to make the code less verbose, but I didn't do that because I like the fact that the current code makes uses of `eval` a bit more obvious (since these behave differently with lexical scoping than with lexical binding, it seemed worthwhile). - Removed the use of `eval` in `org-store-agenda-views` which was only placed there in order to use a macro before it's defined (it would have been simpler/cleaner to just move that functions *after* the macro, but with the new code the problem doesn't occur any more anyway). - Replaced a few `(lambda...) with actual closures. Detailed changes follow: (date, entry): Don't declare as being globally dynbound. (org-agenda-format-date-aligned): Remove unused var `weekyear`. (org-agenda-mode): `run-mode-hooks` is always available nowadays. (org-agenda-undo): Remove unused var `last-undo-buffer`. (org-agenda): Rename arg to `keys` and then dyn-bind it as `org-keys`. Remove unused vars `buf` and `key`. (org-agenda): Use `pcase` and `cl-progv` instead of `org-let`. (org-let, org-let2): Mark as obsolete. (org-agenda-run-series): Use `cl-progv` instead of `org-let` and `org-let2`. (org-agenda-run-series): New function. (org--batch-agenda): New function extracted from `org-batch-agenda`. (org-batch-agenda): Use it. (org--batch-agenda-csv): New function extracted from `org-batch-agenda-csv`. (org-batch-agenda-csv): Use it. (org--batch-store-agenda-views): New function, extracted from `org-batch-store-agenda-views`. (org-store-agenda-views, org-batch-store-agenda-views): Use it. (org--batch-store-agenda-views): Use `cl-progv` instead of `org-eval-in-environment`. (org-agenda-write): Use `cl-progv` instead of `org-let`. Use `with-current-buffer`. (org-agenda-filter-any): Use `cl-some` instead of `eval`. (org-agenda-list): Remove unused var `e`. (org-search-view): η-reduce. (crm-separator): Declare var. (org-agenda-skip-if): Remove unused var `beg`. (org-agenda-list-stuck-projects): Use a closure rather than `(lambda..). (diary-modify-entry-list-string-function, diary-file-name-prefix) (diary-display-function): Declare vars. (org-diary): Declare `date` and `entry` as dynbound. (org-agenda-get-day-entries): Use `org-dlet`. (org-agenda-get-timestamps, org-agenda-get-progress) (org-agenda-get-deadlines, org-agenda-get-scheduled, org-agenda-get-blocks): Declare `date` as dynbound. (org-agenda-get-sexps, org-class): Declare `date` and `entry` as dynbound. (org-agenda-format-item): Declare the vars mentioned in `org-compile-prefix-format` as dyn-bound. Also binding `extra`, suggested by Kyle Meyer . (org-compile-prefix-format): Remove unused var `e`. Use `member` rather than or+equal. (org-set-sorting-strategy): Minor simplification. (org-entries-lessp): Use `org-dlet`. (org-agenda-redo): Declare var `org-agenda-tag-filter-while-redo`. (org-agenda-redo): Use `cl-progv` rather than `org-let`. (org-agenda-filter): Remove unused var `rpl-fn`. Use `org-pushnew-to-end` to replace `add-to-list` on lexical var. (org-agenda-filter-by-tag): Remove unused var `n`. (org-agenda-filter-apply): Use `org-dlet`. (org-agenda-compute-starting-span): Remove unused var `dg`. (org-agenda-forward-block): Remove unused var `pos`. (org-archive-from-agenda): Declare var. (org-agenda-refile): Remove unused var `pos`. (org-agenda-headline-snapshot-before-repeat): Declare var. (org-agenda-todo): Remove redundant use of `bound-and-true-p`. (org-agenda-add-note): Remove unused var `hdmarker` and unused `arg`. (org-agenda-change-all-lines): Remove unused var `pl`. (org-agenda-priority): Remove unused var `marker`. (org-agenda-set-effort): Remove unused var `newhead`. (org-agenda-schedule): Remove unused var `type`. (org-agenda-clock-cancel): Remove unused `arg`. (org-agenda-execute-calendar-command): Use `org-dlet`.
Re: Using lexical-binding
Stefan Monnier writes: > Should I send a rebased patch for inclusion Yes, please. > or do you want to give more time for people to try it out? My guess is that we won't hear much more without bringing the changes into master, and I'm in favor of doing so given that Marco and I have both used it for a good amount of time without finding issues. (Thanks, Marco, for trying it out.) Thank you.
Re: Using lexical-binding
Should I send a rebased patch for inclusion or do you want to give more time for people to try it out? Stefan
Re: Using lexical-binding
> Kyle Meyer writes: >> Stefan Monnier writes: >> >>> Since I'm not using it, I can't really test the result in any meaningful >>> way. Furthermore, just like `calendar.el`, it relies on dynamic scoping >>> and `eval` in all kinds of ways, so it's very difficult to be sure the >>> result is "sufficiently similar" to the old behavior not to break some >>> funky use somewhere out there. >> >> I probably don't use many fancy agenda features, but I do work regularly >> from it. Running with these changes throughout today, I didn't notice >> any issues. Within the next few days, I'll try to test some non-default >> settings and more obscure features that I don't use as part of my normal >> workflow, and see if I can find any problems. > > I've continued to run with these changes and still haven't noticed any > problems. I've also tested various features (sticky agendas, block > agendas, option setting from custom commands) and didn't spot anything. > >> I'll also push the current changes to scratch/sm/agenda-lexical with the >> hope that others will test and report back. > > Has anyone else tried this out? I'm using that branch for several days now without any problem. LGTM! I did nothing special for the test though. At least I use column mode, Org habits and interaction with calendar. Thanks! -- Marco
Re: Using lexical-binding
Kyle Meyer writes: > Stefan Monnier writes: > >> Since I'm not using it, I can't really test the result in any meaningful >> way. Furthermore, just like `calendar.el`, it relies on dynamic scoping >> and `eval` in all kinds of ways, so it's very difficult to be sure the >> result is "sufficiently similar" to the old behavior not to break some >> funky use somewhere out there. > > I probably don't use many fancy agenda features, but I do work regularly > from it. Running with these changes throughout today, I didn't notice > any issues. Within the next few days, I'll try to test some non-default > settings and more obscure features that I don't use as part of my normal > workflow, and see if I can find any problems. I've continued to run with these changes and still haven't noticed any problems. I've also tested various features (sticky agendas, block agendas, option setting from custom commands) and didn't spot anything. > I'll also push the current changes to scratch/sm/agenda-lexical with the > hope that others will test and report back. Has anyone else tried this out?
Re: Using lexical-binding
Stefan Monnier writes: >> It looks like add-to-diary-list became an obsolete alias for >> diary-add-to-list in Emacs 23.1 and was removed in Emacs 25.1, >> specifically 3f65970414 (Remove calendar code obsolete since at least >> version 23.1, 2014-10-05). > > Ah, thanks for tracking it down, so I guess we can drop this altogether. > And we can also drop the `condition-case` in `org-diary-default-entry` > because that change in calling convention is even older than the change > of name from `add-to-diary-list` to `diary-add-to-list`. Yes, sounds good. Done in Org's 0b117f72a.
Re: Using lexical-binding
Stefan Monnier writes: > Since I'm not using it, I can't really test the result in any meaningful > way. Furthermore, just like `calendar.el`, it relies on dynamic scoping > and `eval` in all kinds of ways, so it's very difficult to be sure the > result is "sufficiently similar" to the old behavior not to break some > funky use somewhere out there. I probably don't use many fancy agenda features, but I do work regularly from it. Running with these changes throughout today, I didn't notice any issues. Within the next few days, I'll try to test some non-default settings and more obscure features that I don't use as part of my normal workflow, and see if I can find any problems. I'll also push the current changes to scratch/sm/agenda-lexical with the hope that others will test and report back.
Re: Using lexical-binding
> With a quick test of a few main commands, burps in one of four. Excellent, and thanks for the subsequent patch (I don't think I'd have come up with the move of `extra` on my own). >> - I believe I have quashed all the compiler warnings (some had nothing >> to do with lexical scoping), > > Hmm, I wonder why I'm not seeing the ones unrelated to the lexical > scoping change. I don't think there were many of them. As for why there were some: 1- the change away from `org-let` and friends causes some code to become visible to the compiler (it was hidden behind the "eval wall" until then). 2- I have some extra warnings in my local Emacs. >> except for a reference to the function `add-to-diary-list` which I >> can't find anywhere (is it some old function that has disappeared, >> maybe?). > > It looks like add-to-diary-list became an obsolete alias for > diary-add-to-list in Emacs 23.1 and was removed in Emacs 25.1, > specifically 3f65970414 (Remove calendar code obsolete since at least > version 23.1, 2014-10-05). Ah, thanks for tracking it down, so I guess we can drop this altogether. And we can also drop the `condition-case` in `org-diary-default-entry` because that change in calling convention is even older than the change of name from `add-to-diary-list` to `diary-add-to-list`. Stefan
Re: Using lexical-binding
just a thanks to maintainers of emacs and org including those of you fixing this and those who wrote the tests. i had no idea org wasn't fully lexical yet. i look forward to whatever good that brings. On 2/23/21, Kyle Meyer wrote: > Kyle Meyer writes: > >> Stefan Monnier writes: > [...] >> ;; (org-agenda-list); fails: void-variable date >> >> There are also some `make test' failures: >> >> 7 unexpected results: >> FAILED test-org-agenda/diary-inclusion >> FAILED test-org-agenda/empty >> FAILED test-org-agenda/one-line >> FAILED test-org-agenda/scheduled-non-todo >> FAILED test-org-agenda/set-priority >> FAILED test-org-agenda/sticky-agenda-name >> FAILED test-org-agenda/sticky-agenda-name-after-reload >> >>> or "pretends everything is fine but doesn't do the right thing any >>> more", or (even better) actual feedback about the code itself and the >>> approach(es) I chose to use. >> >> While I'm not sure I can provide any useful feedback about approaches, >> I'll see if I can tweak your patch to resolve the org-agenda-list >> failure or any of the above test failures. > > With the changes below on top of your patch, the simple org-agenda-list > call from above works and the test failures are gone. > > diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el > index 16ec70c77..81409d6ac 100644 > --- a/lisp/org-agenda.el > +++ b/lisp/org-agenda.el > @@ -5448,27 +5448,29 @@ (defun org-agenda-get-day-entries (file date > args) > (setf args (cons :deadline* (delq :deadline* args) > ;; Collect list of headlines. Return them flattened. > (let ((case-fold-search nil) results deadlines) > - (dolist (arg args (apply #'nconc (nreverse results))) > - (pcase arg > - ((and :todo (guard (org-agenda-today-p date))) > -(push (org-agenda-get-todos) results)) > - (:timestamp > -(push (org-agenda-get-blocks) results) > -(push (org-agenda-get-timestamps deadlines) results)) > - (:sexp > -(push (org-agenda-get-sexps) results)) > - (:scheduled > -(push (org-agenda-get-scheduled deadlines) results)) > - (:scheduled* > -(push (org-agenda-get-scheduled deadlines t) results)) > - (:closed > -(push (org-agenda-get-progress) results)) > - (:deadline > -(setf deadlines (org-agenda-get-deadlines)) > -(push deadlines results)) > - (:deadline* > -(setf deadlines (org-agenda-get-deadlines t)) > -(push deadlines results))) > + (org-dlet > + ((date date)) > + (dolist (arg args (apply #'nconc (nreverse results))) > + (pcase arg > + ((and :todo (guard (org-agenda-today-p date))) > + (push (org-agenda-get-todos) results)) > + (:timestamp > + (push (org-agenda-get-blocks) results) > + (push (org-agenda-get-timestamps deadlines) results)) > + (:sexp > + (push (org-agenda-get-sexps) results)) > + (:scheduled > + (push (org-agenda-get-scheduled deadlines) results)) > + (:scheduled* > + (push (org-agenda-get-scheduled deadlines t) results)) > + (:closed > + (push (org-agenda-get-progress) results)) > + (:deadline > + (setf deadlines (org-agenda-get-deadlines)) > + (push deadlines results)) > + (:deadline* > + (setf deadlines (org-agenda-get-deadlines t)) > + (push deadlines results > > (defsubst org-em (x y list) >"Is X or Y a member of LIST?" > @@ -6710,6 +6712,7 @@ (defun org-agenda-format-item (extra txt > level category tags dotime > (get-text-property 1 'effort txt))) >(tag (if tags (nth (1- (length tags)) tags) "")) >(time-grid-trailing-characters (nth 2 org-agenda-time-grid)) > + (extra (or (and (not habitp) extra) "")) >time >(ts (when dotime (concat > (if (stringp dotime) dotime "") > @@ -6793,7 +6796,6 @@ (defun org-agenda-format-item (extra txt > level category tags dotime >(concat time-grid-trailing-characters " > ") > time-grid-trailing-characters))) >(t "")) > - extra (or (and (not habitp) extra) "") > category (if (symbolp category) (symbol-name category) category) > level (or level "")) > (if (string-match org-link-bracket-re category) > > -- The Kafka Pandemic Please learn what misopathy is.
Re: Using lexical-binding
Kyle Meyer writes: > Stefan Monnier writes: [...] > ;; (org-agenda-list); fails: void-variable date > > There are also some `make test' failures: > > 7 unexpected results: > FAILED test-org-agenda/diary-inclusion > FAILED test-org-agenda/empty > FAILED test-org-agenda/one-line > FAILED test-org-agenda/scheduled-non-todo > FAILED test-org-agenda/set-priority > FAILED test-org-agenda/sticky-agenda-name > FAILED test-org-agenda/sticky-agenda-name-after-reload > >> or "pretends everything is fine but doesn't do the right thing any >> more", or (even better) actual feedback about the code itself and the >> approach(es) I chose to use. > > While I'm not sure I can provide any useful feedback about approaches, > I'll see if I can tweak your patch to resolve the org-agenda-list > failure or any of the above test failures. With the changes below on top of your patch, the simple org-agenda-list call from above works and the test failures are gone. diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el index 16ec70c77..81409d6ac 100644 --- a/lisp/org-agenda.el +++ b/lisp/org-agenda.el @@ -5448,27 +5448,29 @@ (defun org-agenda-get-day-entries (file date args) (setf args (cons :deadline* (delq :deadline* args) ;; Collect list of headlines. Return them flattened. (let ((case-fold-search nil) results deadlines) - (dolist (arg args (apply #'nconc (nreverse results))) - (pcase arg - ((and :todo (guard (org-agenda-today-p date))) - (push (org-agenda-get-todos) results)) - (:timestamp - (push (org-agenda-get-blocks) results) - (push (org-agenda-get-timestamps deadlines) results)) - (:sexp - (push (org-agenda-get-sexps) results)) - (:scheduled - (push (org-agenda-get-scheduled deadlines) results)) - (:scheduled* - (push (org-agenda-get-scheduled deadlines t) results)) - (:closed - (push (org-agenda-get-progress) results)) - (:deadline - (setf deadlines (org-agenda-get-deadlines)) - (push deadlines results)) - (:deadline* - (setf deadlines (org-agenda-get-deadlines t)) - (push deadlines results))) + (org-dlet + ((date date)) + (dolist (arg args (apply #'nconc (nreverse results))) + (pcase arg + ((and :todo (guard (org-agenda-today-p date))) +(push (org-agenda-get-todos) results)) + (:timestamp +(push (org-agenda-get-blocks) results) +(push (org-agenda-get-timestamps deadlines) results)) + (:sexp +(push (org-agenda-get-sexps) results)) + (:scheduled +(push (org-agenda-get-scheduled deadlines) results)) + (:scheduled* +(push (org-agenda-get-scheduled deadlines t) results)) + (:closed +(push (org-agenda-get-progress) results)) + (:deadline +(setf deadlines (org-agenda-get-deadlines)) +(push deadlines results)) + (:deadline* +(setf deadlines (org-agenda-get-deadlines t)) +(push deadlines results (defsubst org-em (x y list) "Is X or Y a member of LIST?" @@ -6710,6 +6712,7 @@ (defun org-agenda-format-item (extra txt level category tags dotime (get-text-property 1 'effort txt))) (tag (if tags (nth (1- (length tags)) tags) "")) (time-grid-trailing-characters (nth 2 org-agenda-time-grid)) +(extra (or (and (not habitp) extra) "")) time (ts (when dotime (concat (if (stringp dotime) dotime "") @@ -6793,7 +6796,6 @@ (defun org-agenda-format-item (extra txt level category tags dotime (concat time-grid-trailing-characters " ") time-grid-trailing-characters))) (t "")) - extra (or (and (not habitp) extra) "") category (if (symbolp category) (symbol-name category) category) level (or level "")) (if (string-match org-link-bracket-re category)
Re: Using lexical-binding
Stefan Monnier writes:
> As part of the on-going work to use lexical-binding in all the files
> bundled with Emacs, I took a stab at converting org-agenda.el to
> lexical-binding.
Thank you.
> [...]
> Anyway, here's my first cut (the patch is made against the head of
> Org's `master` rather than Emacs's `master`, since I suspect that could
> make things easier for you). The commit message is basically empty
> because it's not intended to be installed yet. I'm instead hoping for
> some feedback, such as "tried it, works" or "burps all over the place",
With a quick test of a few main commands, burps in one of four.
Contents /tmp/scratch.org:
--8<---cut here---start->8---
* TODO a :t:
SCHEDULED: <2021-02-23 Tue>
foo
* DONE b
* TODO c
DEADLINE: <2021-02-23 Tue>
--8<---cut here---end--->8---
Running with emacs 27.1 and -Q:
(require 'org-agenda)
(setq org-agenda-files '("/tmp/scratch.org"))
(global-set-key (kbd "C-c a") #'org-agenda)
;; Commands:
;; (org-todo-list) ; works
;; (org-search-view nil "foo") ; works
;; (org-tags-view nil "+t") ; works
;; (org-agenda-list); fails: void-variable date
There are also some `make test' failures:
7 unexpected results:
FAILED test-org-agenda/diary-inclusion
FAILED test-org-agenda/empty
FAILED test-org-agenda/one-line
FAILED test-org-agenda/scheduled-non-todo
FAILED test-org-agenda/set-priority
FAILED test-org-agenda/sticky-agenda-name
FAILED test-org-agenda/sticky-agenda-name-after-reload
> or "pretends everything is fine but doesn't do the right thing any
> more", or (even better) actual feedback about the code itself and the
> approach(es) I chose to use.
While I'm not sure I can provide any useful feedback about approaches,
I'll see if I can tweak your patch to resolve the org-agenda-list
failure or any of the above test failures.
> - I believe I have quashed all the compiler warnings (some had nothing
> to do with lexical scoping),
Hmm, I wonder why I'm not seeing the ones unrelated to the lexical
scoping change. `make compile' and `make single' are quiet for me on
Org's current master (d21d200bc) with Emacs 27. If I use an Emacs built
from a recent Emacs commit (6172454ff3), I see a couple of "docstring
wider than 80 characters" warnings (will fix), but nothing in
org-agenda.el.
> except for a reference to the function `add-to-diary-list` which I
> can't find anywhere (is it some old function that has disappeared,
> maybe?).
It looks like add-to-diary-list became an obsolete alias for
diary-add-to-list in Emacs 23.1 and was removed in Emacs 25.1,
specifically 3f65970414 (Remove calendar code obsolete since at least
version 23.1, 2014-10-05).
Using lexical-binding
As part of the on-going work to use lexical-binding in all the files bundled with Emacs, I took a stab at converting org-agenda.el to lexical-binding. Since I'm not using it, I can't really test the result in any meaningful way. Furthermore, just like `calendar.el`, it relies on dynamic scoping and `eval` in all kinds of ways, so it's very difficult to be sure the result is "sufficiently similar" to the old behavior not to break some funky use somewhere out there. Anyway, here's my first cut (the patch is made against the head of Org's `master` rather than Emacs's `master`, since I suspect that could make things easier for you). The commit message is basically empty because it's not intended to be installed yet. I'm instead hoping for some feedback, such as "tried it, works" or "burps all over the place", or "pretends everything is fine but doesn't do the right thing any more", or (even better) actual feedback about the code itself and the approach(es) I chose to use. Stefan - Removed the global (defvar date) and (defvar entry) so as not to conflict with function arguments of that name. Instead I added such `defvar`s in the body of each of the functions where it seemed needed. - I believe I have quashed all the compiler warnings (some had nothing to do with lexical scoping), except for a reference to the function `add-to-diary-list` which I can't find anywhere (is it some old function that has disappeared, maybe?). - Added an `org-dlet` macro, just like I had done for `calendar-dlet`, but I also use `defvar` "manually" at some places, when splitting an existing `let` into a mix of `let`s and `dlet`s seemed too much trouble. - Removed uses of `org-let and `prg-let2` not only because I consider them offensive to my sense of aesthetics but also because they're basically incompatible with lexical scoping. I replaced them with uses of `cl-progv` which are a bit more verbose. Maybe we should define some `org-progv` macro on top of `cl-progv` to make the code less verbose, but I didn't do that because I like the fact that the current code makes uses of `eval` a bit more obvious (since these behave differently with lexical scoping than with lexical binding, it seemed worthwhile). - Removed the use of `eval` in `org-store-agenda-views` which was only placed there in order to use a macro before it's defined (it would have been simpler/cleaner to just move that functions *after* the macro, but with the new code the problem doesn't occur any more anyway). - Replaced a few `(lambda...) with actual closures. >From d34f993044ee817f7ee18342bcc686285329bea5 Mon Sep 17 00:00:00 2001 From: Stefan Monnier Date: Tue, 23 Feb 2021 15:47:29 -0500 Subject: [PATCH] * org-agenda.el: First attempt at using `lexical-binding` --- .gitignore | 6 + doc/Makefile | 14 +- lisp/org-agenda.el | 827 + lisp/org-macs.el | 41 ++- 4 files changed, 502 insertions(+), 386 deletions(-) diff --git a/.gitignore b/.gitignore index 1a72cc20b0..4bb81c359b 100644 --- a/.gitignore +++ b/.gitignore @@ -49,6 +49,12 @@ local*.mk mk/x11idle ChangeLog +# Files generated during `make packages/org` in a clone of `elpa.git`. + +/org-pkg.el +/org-autoloads.el +/lisp/org-autoloads.el + # texi2pdf --tidy doc/*.t2d diff --git a/doc/Makefile b/doc/Makefile index 96fda14454..dc6882927e 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -28,9 +28,9 @@ guide:: orgguide.texi org-version.inc endif org.texi orgguide.texi: org-manual.org org-guide.org - $(BATCH) \ - --eval '(add-to-list '"'"'load-path "../lisp")' \ - --eval '(load "../mk/org-fixup.el")' \ + $(BATCH) \ + --eval '(add-to-list `load-path "../lisp")' \ + --eval '(load "../mk/org-fixup.el")' \ --eval '(org-make-manuals)' org-version.inc: org.texi @@ -88,8 +88,8 @@ ifneq ($(SERVERMK),) endif %_letter.tex: %.tex - $(BATCH) \ - --eval '(add-to-list '"'"'load-path "../lisp")' \ - --eval '(load "org-compat.el")' \ - --eval '(load "../mk/org-fixup.el")' \ + $(BATCH) \ + --eval '(add-to-list `load-path "../lisp")' \ + --eval '(load "org-compat.el")' \ + --eval '(load "../mk/org-fixup.el")' \ --eval '(org-make-letterformat "$(= iso-week 52)) - (1- year)) - ((and (= month 12) (<= iso-week 1)) - (1+ year)) - (t year))) + ;; (weekyear (cond ((and (= month 1) (>= iso-week 52)) + ;; (1- year)) + ;; ((and (= month 12) (<= iso-week 1)) + ;; (1+ year)) + ;; (t year))) (weekstring (if (= day-of-week 1) (format " W%02d" iso-week) ""))) @@ -2269,7 +2271,7 @@ The following commands are available: (setq-local org-agenda-this-buffer-is-sti
