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

Previous Next

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: Thu, 09 May 2024 16:59:33 -0700

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.

>> Hope this is useful!
>
> Very helpful yes.
>
> I've also noticed that the change I proposed - moving the part of the
> code which modifies match variable to the beginning of the
> `gnus-summary-score-entry' function is a bad idea because we're
> modifying input argument to the function. It would mean that if the user
> called the function non interactively (without prompt):
>
> (gnus-summary-score-entry "date" "55" '< 1000 nil)
>
> then it would change "55" to different number based on article age. I've
> moved that code back where it was. I attach the patch below.

Alex has commit access to Emacs -- Alex would you please commit and
close the bug once the two of you are happy with this?

Previous Next

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