bug#70134: [PATCH] Show all date options when adding Gnus scores interactively

[Eric Abrahamsen <eric@ericabrahamsen.net>]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Jakub Ječmínek via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
>
>> "Alex Bochannek" <alex@bochannek.com> writes:
>>
>>> I finally had some time to look at these changes, apologies for the
>>> delay.
>>
>> Thank you very much for your time and all your comments.
>
> Thanks both to Alex for this response, and Jakub for your earlier
> explanation of what's going on. I'll try to add some more docstrings
> after this is resolved.
>
>>> I like the approach and tested them out. The legal-types change in
>>> gnus-summary-increase-score is straightforward and makes sense to me. I
>>> have one stylistic comment: (nthcdr 3 s) seems to be easier to read to
>>> me than (cdddr s), but I have no strong opinions on that.
>>
>> Thank you, I used (nthcdr 3 s) instead.
>>
>>> My suspicion is that this started out as a copy of the integer
>>> comparison right above that code and I never cleaned it up. Yes, feel
>>> free to simplify, I don't remember any good reason why it needs to pick
>>> apart a list. It also seems perfectly fine to remove the (eq type
>>> 'after) etc. stuff, it's not necessary anymore.
>>
>> I've adjusted only the age scoring part (not the integer comparison)
>> because I did not studied that part of the code and there's still
>> possibility that it is needed somehow.
>>
>>> The rest of the changes in gnus-summary-score-entry look good. I think
>>> some more help text or additional documentation about the defaults would
>>> be useful. It gets a bit confusing what you are prompted for. Having
>>> said that, I like the idea of pulling a default date from the current
>>> message, it just surprised me.
>>
>> I've added a comment explaining the changes I made to the date
>> prompt. If you feel like the code needs more comments, please pinpoint
>> where and I will add them.
>>
>>> I also think I might have found a bug in how the dates are written out
>>> to the SCORE file. I interactively increased the score in the order of
>>> <, r, n, b, and n as you can see below. Only the b, a, and n entries get
>>> converted to the list format with the un-evaluated gnus-time after
>>> another entry is written. Meaning the second and third entry below, the
>>> "before" and "at," looked just like the topmost "at" entry before the
>>> following entry was written.
>>
>> I've found the reason why it happens and found a solution. The problem
>> is in the `gnus-date-get-time' macro. This macro accepts a single
>> argument - date - and returns a different one - time - with text
>> property added. However, this macro is written in such way that it
>> modifies the input argument as well. We can fix it by adding `copy-sequence'
>> function to the let form.
>
> This is grim, thanks for finding it. I'm inclined to fix this first in a
> stand-alone commit.

And sure enough, the modification of the string is the point -- it's a
cache! From gnus-sum.el:

; Since this is called not only to sort the top-level threads, but
; also in recursive sorts to order the articles within a thread, each
; article will be processed many times.  Thus it speeds things up
; quite a bit to use gnus-date-get-time, which caches the time value.
(defun gnus-thread-latest-date (thread)
  "Return the highest article date in THREAD."
  (apply #'max
	 (mapcar (lambda (header) (float-time
				   (gnus-date-get-time
				    (mail-header-date header))))
		 (flatten-tree thread))))

Can we strip properties around the call, maybe?