GNU bug report logs - #70134
[PATCH] Show all date options when adding Gnus scores interactively

Previous Next

Package: emacs;

Reported by: Jakub Ječmínek <kuba <at> kubajecminek.cz>

Date: Mon, 1 Apr 2024 21:45:01 UTC

Severity: normal

Tags: patch

Done: Jakub Ječmínek <kuba <at> kubajecminek.cz>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Alex Bochannek <alex <at> bochannek.com>
Cc: 70134 <at> debbugs.gnu.org, Jakub Ječmínek <kuba <at> kubajecminek.cz>, eliz <at> gnu.org, larsi <at> gnus.org, Richard Stallman <rms <at> gnu.org>
Subject: bug#70134: [PATCH] Show all date options when adding Gnus scores interactively
Date: Fri, 10 May 2024 13:04:39 -0700
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> Jakub Ječmínek via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs <at> gnu.org> writes:
>
>> "Alex Bochannek" <alex <at> 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?




This bug report was last modified 346 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.