bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#39980: [PATCH] gnus-shorten-url: Improve and avoid args-out-of-range


From: Štěpán Němec
Subject: bug#39980: [PATCH] gnus-shorten-url: Improve and avoid args-out-of-range error
Date: Sun, 12 Apr 2020 13:47:20 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)

On Sun, 12 Apr 2020 14:05:40 +0300
Eli Zaretskii wrote:

>> The revised patch is upthread
>> (<https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-03/msg01100.html>)
>
> That's the one I was talking about: it includes changes that aren't
> needed to fix the original bug.  Those additions make the change
> cleaner, but we are way past cleanup time on the release branch.
>
>> There are no changes in the patch that are not necessary for fixing the
>> problem. The ediff changes (only adjusting callers) are necessitated by
>> moving the helper function to subr-x.el, so that Gnus (or anything else)
>> can use it, too.
>
> The changes in subr-x and in ediff are unnecessary.  Let's make
> changes only in gnus-summary-browse-url, OK?  The full changeset can
> go to master.

You mean instead of reusing an existing function, fix `gnus-shorten-url'
without using it? I can only see disadvantages here: having to maintain
two separate versions, reinventing the wheel (and possibly introducing
bugs that way, which is how all this started)... are you sure you
couldn't be persuaded otherwise? Surely just renaming a function and
adjusting callers should be safer for the release branch, too?

> Btw, this part of the commit log is inaccurate:
>
>   * lisp/vc/ediff-init.el
>   (ediff-truncate-string-left): Rename to 'string-truncate-left'.
>
> And this one names the wrong function:
>
>   * lisp/vc/ediff-mult.el (ediff-meta-insert-file-info1) 
> (ediff-draw-dir-diffs):
>   'ediff-draw-dir-diffs' renamed to 'string-truncate-left'.
>
> And finally, the format of the last entry is slightly off the mark.
> It should look like this:
>
>   * lisp/vc/ediff-mult.el (ediff-meta-insert-file-info1)
>   (ediff-draw-dir-diffs):
>
> That is, the second parentheses pair should be on a new line.

Thank you, how about:

* lisp/vc/ediff-init.el (ediff-truncate-string-left): Rename to
'string-truncate-left' and move...
* lisp/emacs-lisp/subr-x.el (string-truncate-left): ...here.
* lisp/vc/ediff-mult.el (ediff-meta-insert-file-info1)
(ediff-draw-dir-diffs): Adjust callers.
* lisp/gnus/gnus-sum.el (gnus-shorten-url): Fix args-out-of-range
error, don't drop #fragments, use 'string-truncate-left'.

Or even compacting the first three items to:

* lisp/vc/ediff-init.el (ediff-truncate-string-left): Rename to
'string-truncate-left' and move...
* lisp/emacs-lisp/subr-x.el (string-truncate-left): ...here.  All callers
changed.

as suggested in https://www.gnu.org/prep/standards/standards.html#Simple-Changes

-- 
Štěpán





reply via email to

[Prev in Thread] Current Thread [Next in Thread]