unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eric Abrahamsen <eric@ericabrahamsen.net>
To: Alex Bochannek <alex@bochannek.com>
Cc: 70134@debbugs.gnu.org, "Jakub Ječmínek" <kuba@kubajecminek.cz>,
	eliz@gnu.org, larsi@gnus.org, "Richard Stallman" <rms@gnu.org>
Subject: bug#70134: [PATCH] Show all date options when adding Gnus scores interactively
Date: Thu, 09 May 2024 16:59:33 -0700	[thread overview]
Message-ID: <87bk5esfiy.fsf@ericabrahamsen.net> (raw)
In-Reply-To: <87bk5eu78m.fsf@kubajecminek.cz> ("Jakub Ječmínek via \"Bug reports for GNU Emacs, the Swiss army knife of text editors\""'s message of "Thu, 09 May 2024 19:15:41 +0000")

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.

>> 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?





  reply	other threads:[~2024-05-09 23:59 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-01 21:44 bug#70134: [PATCH] Show all date options when adding Gnus scores interactively Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-13  7:58 ` Eli Zaretskii
2024-04-22  3:33   ` Eric Abrahamsen
2024-04-24 22:52     ` Richard Stallman
2024-04-24 23:08       ` Alex Bochannek
2024-05-01 19:27         ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-07  2:53           ` Alex Bochannek
2024-05-09 19:15             ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-09 23:59               ` Eric Abrahamsen [this message]
2024-05-10 20:04                 ` Eric Abrahamsen
2024-05-10 20:38                   ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-10 21:27                     ` Eric Abrahamsen
2024-05-14  2:00                     ` Alex Bochannek
2024-05-14 14:52                       ` Eric Abrahamsen
2024-05-14 18:43                         ` Alex Bochannek
2024-05-14 19:57                           ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-16 14:15                             ` Eric Abrahamsen
2024-05-16 19:40                               ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-22 21:34 ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-22 23:30   ` Alex Bochannek
2024-05-23  2:50     ` Eric Abrahamsen
2024-05-26 14:22       ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-26 22:09 ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-29 21:23 ` Jakub Ječmínek via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-30  0:16   ` Alex Bochannek
2024-05-30  9:01     ` Andrea Corallo
2024-05-30 16:30       ` Alex Bochannek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bk5esfiy.fsf@ericabrahamsen.net \
    --to=eric@ericabrahamsen.net \
    --cc=70134@debbugs.gnu.org \
    --cc=alex@bochannek.com \
    --cc=eliz@gnu.org \
    --cc=kuba@kubajecminek.cz \
    --cc=larsi@gnus.org \
    --cc=rms@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).