Subject: bug#12396: 24.2.50; [PATCH] extend usability of gnus-read-ephemeral-emacs-bug-group

From: Stephen Berman <[email-masked]>
Date: Wed, 12 Sep 2012 01:11:04 +0200

On Tue, 11 Sep 2012 22:01:34 +0300 Juri Linkov <[email-masked]> wrote:

>>> It makes sense, but it's kinda not the way these function in Emacs
>>> usually work.  `M-x find-file-at-point', `M-x man', etc, all react to
>>> the thing under point.  And that's a very self-evident user interface.
>>> Put point at the thing you're interested in, and then issue a command.
>>
>> I was annoyed enough times by having to navigate to the bug number to
>> avoid typing it in that I decided to code up a way to avoid having to do
>> that; having it in Emacs might spare others such annoyance. :-)
>
> I agree with Lars that getting a default value from distant parts
> of the buffer is non-standard behavior for thing-at-point.

But my patch uses re-search-forward for getting the default value from
"distant parts of the buffer"; its use of thing-at-point is completely
standard.

> It might cause more annoyance when it will do not what the user expects.
> For instance, if it will get a bug number far away from point, then
> the user might expect that it will also get a bug number on the next line
> at the same column position, etc.  IOW, I don't oppose such a change
> only if its logic would be simple and intuitive to users.

Well, the use case that motivated my first patch is grabbing a bug
number in the subject line of a mail or posting, so the distance
involved is rather small.  Admittedly, the extension to ChangeLogs could
involve several lines between point and the bug number, which my patch
does not take into account.

> Also moving argument reading out of the `interactive' specification
> in `gnus-read-ephemeral-emacs-bug-group' is a change for the worse.

This was necessitated by the optional numerical prefix argument, which
is passed to gnus-get-emacs-bug-number, which supplies the default for
the argument reading; AFAICS it is impossible to have one argument
determine another argument within the same interactive spec.

> However, I think that the second part of your proposal is very useful.
>
> Do you think `bug-reference-push-button-action' can be a function?
> So users could set it to `browse-url' or `gnus-read-ephemeral-emacs-bug-group'

Certainly; see below.

> (these values still need const tags like in your patch)

Actually, function-item uses the doc string, so tags aren't needed.

> or any other function.  Also I wonder why you don't call
> `gnus-read-ephemeral-emacs-bug-group' with a bug number argument
> so users don't need to confirm it with RET.

Well, my second patch assumed my first patch, which changes
g-r-e-e-b-g's signature, so the bug number isn't passed as an argument.
However, I agree it is bad to request confirmation on clicking the
button in this case.  This appears to argue against the change in the
interactive spec.  I'll have to give it some thought.  In the mean time,
since my proposed change to g-r-e-e-b-g doesn't seem to be gaining
support and the implementation is problematical (though I still think
the feature is worth having), here's a reworking of the second patch
along the lines you suggested, using the existing g-r-e-e-b-g instead of
my proposed change.  I haven't yet made it a patch against the trunk
because I only tested it briefly, but it seems to work:

(defun bug-reference-browse-url (pos)
  "Visit the bug's URL in a web browser."
  (dolist (o (overlays-at pos))
    ;; It should only be possible to have one URL overlay.
    (let ((url (overlay-get o 'bug-reference-url)))
      (when url
	(browse-url url)))))
			      
(defun bug-reference-gnus-read-ephemeral-emacs-bug-group (pos)
  "Read the bug thread in a Gnus ephemeral group."
  (dolist (o (overlays-at pos))
    ;; It should only be possible to have one URL overlay.
    (let* ((url (overlay-get o 'bug-reference-url))
	   (num (when (string-match "http://debbugs\\.gnu\\.org/\\([0-9]+\\)$" url)
		  (string-to-number (match-string 1 url)))))
      (when num (gnus-read-ephemeral-emacs-bug-group num)))))

(defcustom bug-reference-push-button-action 'bug-reference-browse-url
  "Function called on clicking a bug reference button."
  :group 'bug-reference
  :type '(radio (function-item bug-reference-browse-url)
		(function-item bug-reference-gnus-read-ephemeral-emacs-bug-group)
		function))

(defun bug-reference-push-button (&optional pos _use-mouse-action)
  "Take appropriate action on the bug reference at POS.
The action depends on the value of the user option
`bug-reference-push-button-action'."
  (interactive
   (list (if (integerp last-command-event) (point) last-command-event)))
  (if (and (not (integerp pos)) (eventp pos))
      ;; POS is a mouse event; switch to the proper window/buffer
      (let ((posn (event-start pos)))
	(with-current-buffer (window-buffer (posn-window posn))
	  (bug-reference-push-button (posn-point posn) t)))
    ;; POS is just normal position.
    (funcall bug-reference-push-button-action pos)))

> BTW, bug-reference.el could be improved to support also links
> to revision numbers like "revno:109985" that opens an URL like
> http://bzr.savannah.gnu.org/lh/emacs/trunk/revision/109985

That would be useful, though it's going beyond bug references.

Steve Berman