Hey Jacob,

Thank you so much for the fast turn around and the high quality patch!

I apologize for the delay.  Turns out my CI was broken.  You did not break any
tests.

"Jacob S. Gordon" <[email protected]> writes:

> Hey Morgan,
>
> Thanks for your diligence!  Attached is a patch that tries to address
> the errors, but I don’t have Emacs 28 setup to test (one of these days
> I’ll have to ask about your multi-Emacs CI setup).

It's based on guix but it is an entire rabbit hole that seems to break more
often then I'd like.  I don't mind one bit doing the testing for you.  I'd much
rather you spent your time on more fruitful endeavors like adding this feature!

> On 2026-05-18 20:40, Morgan Smith wrote:
>> org-agenda.el:10303:1: Warning: variable `_' not left unused
>> org-agenda.el:10303:1: Warning: variable `_' not left unused
>
> This seems similar to [bug#69108] fixed by ‘e680827e814’ for Emacs 30.
> I’ve removed the ‘_’ bindings in case that’s it.
>
> [bug#69108]<https://lists.gnu.org/archive/html/bug-gnu-emacs/2024-02/msg00723.html>

Yes that was exactly it!  It looks like they might try to deprecate this syntax
of `when-let*' as I can see in this quote from the docstring of `if-let*'

#+begin_quote
An older form for entries of VARLIST is also supported, where SYMBOL is
omitted, i.e. (VALUEFORM).  This means the same as (_ VALUEFORM).
This form is not recommended because many Lisp programmers find it
significantly less readable.  A future release of Emacs may introduce a
byte-compiler warning for uses of (VALUEFORM) in VARLIST.
#+end_quote

That being said, this patch is perfectly readable.  I doubt anyone will
assume you're assigning to `eq' or `not'.  I started going down the
rabbit hole of trying to rewrite it with `seq-reduce' (which seems like
a useful function but I've never managed to find a place to use it) but
I'm trying to commit to less rabbit holes so I purposefully gave up.

>> In org-agenda-clock-goto:
>> org-agenda.el:10312:12: Warning: sort called with 3 arguments, but accepts
>>     only 2
>
> Oh interesting, I didn’t know ‘sort’ was different pre-Emacs 30
> (‘ae5f2c02bd2’): ‘(sort SEQ LESSP)’.  I’ve adapted to the compatible
> calling convention.

Looks good to me!

I've run it through CI Morgan with the following results:

Tests pass on emacs 29, 30.2, and a recent emacs/master build.
Tests pass with TZ set to UTC, Europe/Istanbul, and America/New_York.

4 test failures on Emacs 28 but that's just because CI Morgan is still a little
broken.

Applied, as commit 5261e65d5

Thank you very much!!

Reply via email to