Subject: Re: bug#59458: [PATCH] Fix tracing for advanced scoring

From: Eric Abrahamsen <[email-masked]>
Date: Mon, 11 Sep 2023 17:07:16 -0700

On 09/07/23 14:07 PM, Stefan Kangas wrote:
> Eric Abrahamsen <[email-masked]> writes:
>
>> Eli Zaretskii <[email-masked]> writes:
>>
>>>> Cc: Łukasz Stelmach <[email-masked]>
>>>> From: Łukasz Stelmach <[email-masked]>
>>>> Date: Mon, 21 Nov 2022 22:30:55 +0100
>>>>
>>>> * lisp/gnus/gnus-logic.el (gnus-score-advanced): Move the tracing
>>>> code outside of if so it's executed for both branches.
>>
>> I'm not very familiar with this code (this is actually the first I'm
>> hearing of gnus-logic.el), so I hope Lars will chime in. It makes sense
>> that tracing should happen whether or not the rule matched? But what
>> about the sexp before that? Would we be pushing the mail-header-number
>> and new score to `gnus-newsgroup-score' only if the rule *wasn't*
>> successful?
>>
>> I think this one should wait for Lars. If we don't hear from him and
>> it's holding things up, I can look more closely.
>
> Eric,
>
> It would be great if you could help review this.  Thanks in advance.

I've taken a closer look, and I do think this is okay. The `when' after
the `dolist' only fires if the rule matches, the `if' is only there to
see if this article has been previously scored or not. So moving the
"(when trace" up to the top-level of the containing `when' (ie, out of the
`if') does look like the right thing to do.

Eric