Subject: Re: bug#72735: 31.0.50; [PATCH] Make more bug-reference variables customizeable

From: Tassilo Horn <[email-masked]>
Date: Wed, 12 Feb 2025 07:25:50 +0100

Björn Bidar <[email-masked]> writes:

Hi Björn,

>> As Eli already mentioned, bug-reference-url-format is usually set via
>> a file local variables section or programatically so neither a
>> defcustom nor a default value makes sense.
>>
>> Wrt. bug-reference-setup-from-mail-alist,
>> bug-reference-setup-from-irc-alist, and
>> bug-reference-setup-from-vc-alist: yes, they could be defcustoms but
>> their entries can all (and are likely to) contain functions which are
>> hard to specify in the defcustom interface.  I've thought it wouldn't
>> be needed.  After all, bug-reference is a programmer's tool.
>
> Isn't that a weak argument against changing those to a defcustom?

Maybe.  I have no problem with those three being defcustoms.

> With the current default values if not modified it is much harder to
> use them for anything else but the GNU debbugs instance.

No, bug-reference should work out-of-the-box for every project checked
out of the forges in bug-reference-forge-alist.  Well, unless you've set
bug-reference-url-format globally which would turn off the auto-setup.

> Correct me if I'm wrong but all those variables only regular
> expressions and a format string which is something that already has
> been done using defcustom e.g. as in Gnus.

In all three, BUG-REGEXP and URL-FORMAT can be functions as documented
by bug-reference-bug-regexp and bug-reference-url-format.  Oh, and with
bug-reference-setup-from-vc-alist there's only a URL-FORMAT-FN which
must be a function.

> Isn't it the policy that for settings variables there should be
> defcustom variables? E.g. as mentioned in (info "(elisp) Documentation
> Tips"):
>
>>    • When you define a variable that represents an option users might
>>       want to set, use ‘defcustom’.  *Note Defining Variables::.
>
> Or is the purpose of bug-reference mode GNU specific and not intended
> to be used for anything but GNU?

As said, it works out-of-the-box for Github, several Gitlab instances,
codeberg, framagit, salsa.debian.org, and sr.ht.

And adding you company's tracker is quite easy, too.  That's what I use:

--8<---------------cut here---------------start------------->8---
(let ((shd-jira-regexp (concat "\\b\\(\\("
                               (regexp-opt '("ARCA" "BRI" "ECOJ" "KASE" "MOBA"
                                             "PORC" "LOWE" "SABA"))
                               "-[0-9]\\{1,6\\}\\)\\)\\b")))
  (add-to-list 'bug-reference-setup-from-vc-alist
               `("srv-upsource\\.shd\\.lan"
                 ,shd-jira-regexp
                 ,(lambda (_)
                    :dont-hl-value-as-docstring
                    "http://srvjira-and.shd.lan/browse/%s")))

  (add-to-list 'bug-reference-setup-from-mail-alist
               `("SHD" nil ,shd-jira-regexp
                 "http://srvjira-and.shd.lan/browse/%s"))
--8<---------------cut here---------------end--------------->8---

>> Wrt. bug-reference-forge-alist: if it became a defcustom and a user
>> would set it, she wouldn't see updates (like support for some new
>> forge) in its default value anymore because their old saved custom
>> value overrides the new default value.  It's much better to add new
>> entries programatically using add-to-list or push/cl-pushnew.  Of
>> course, one could split the variable in some *-default-alist defvar
>> and a defcustom *-alist where the latter is meant for user
>> customization but I think it's not worth the added complexity here.
>
> Didn't have time to work on this. Besides that the general downside
> with custom is that if the user modifies it they do not get the
> updates to the default value a separate variable similar to
> browse-url-handler and browse-url-defautl-handlers would be an option.

That's what I've said, yes.  And I recall some recent discussion where
it was argued that the browse-url-(default-)handlers split (one for
users, one for emacs/packages) was adding too much complexity.

Bye,
Tassilo