[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#70134: [PATCH] Show all date options when adding Gnus scores interac
From: |
Alex Bochannek |
Subject: |
bug#70134: [PATCH] Show all date options when adding Gnus scores interactively |
Date: |
Mon, 06 May 2024 19:53:35 -0700 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Jakub,
I finally had some time to look at these changes, apologies for the
delay.
Jakub Ječmínek <kuba@kubajecminek.cz> writes:
> "Alex Bochannek" <alex@bochannek.com> writes:
>> As Jakub mentioned, I am the author of that part of the Gnus scoring
>> code. I have not had a chance to look at the proposed patch, but I agree
>> that some explanations would be useful. I can look it over this weekend.
>
> Thank you, your review is appreciated. Please do you happen to remember
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.
> why you used this form
>
> (string= "date" (nth 0 (assoc header gnus-header-index)))
>
> instead of this
>
> (string= header "date")
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.
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 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.
("date"
("20240501" nil nil at)
(#("20240502" 0 1
(gnus-time
(26163 14832)))
nil nil before)
(#("20240503T145117" 0 1
(gnus-time
(26165 23637)))
nil nil at)
(".*T031840" nil nil r)
(2 nil nil <)
> Thanks!
Hope this is useful!
--
Alex.
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Jakub Ječmínek, 2024/05/01
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively,
Alex Bochannek <=
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Jakub Ječmínek, 2024/05/09
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Eric Abrahamsen, 2024/05/09
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Eric Abrahamsen, 2024/05/10
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Jakub Ječmínek, 2024/05/10
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Eric Abrahamsen, 2024/05/10
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Alex Bochannek, 2024/05/13
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Eric Abrahamsen, 2024/05/14
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Alex Bochannek, 2024/05/14
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Jakub Ječmínek, 2024/05/14
- bug#70134: [PATCH] Show all date options when adding Gnus scores interactively, Eric Abrahamsen, 2024/05/16