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

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

bug#39912: [PATCH] Open describe-function links in view-mode (Bug#39912)


From: Stefan Kangas
Subject: bug#39912: [PATCH] Open describe-function links in view-mode (Bug#39912)
Date: Tue, 8 Sep 2020 21:01:38 +0000

"Nicholas Savage" <nick@nicksavage.ca> writes:

> I've written a patch for Bug #39912, as discussed in debbugs. I think I
> have formatted this correctly and have followed CONTRIBUTE as best as I
> could.

Thanks!  I have some comments.

(Someone else will have to help you with getting the assignment process
started.)

> I've also changed it so the actual source code link opens in view-mode
> as well. I don't know if there are ramifications of that, so I'm happy
> to change it if necessary. My thought process was that
> describe-function is used as a reference to the code, not as a point
> of entry for editing functions, so changing that is consistent with
> changing the NEWS links, but I don't know what I don't know.

Actually, I use it to edit the source code all the time.  That's what
that button is mostly used for, in my use.  :-)

So I think we'd better leave that part out.

> * Open describe-function links in view-mode (Bug#39912) to make
> behavior consistent depending on whether or not user has permission
> to edit the files.

The description is okay, but could be shorter (and should mention the
link to NEWS button as discussed above).  I'm also not sure if its
necessary to discuss file permissions here, since those details will be
in the bug report.  Mentioning view-mode should be enough.

Also, this doesn't seem to follow the ChangeLog format.  If you use VC
(or Magit) to edit the files, you can simply put point on the newly
added line and type `C-x 4 a' to get it automatically generated.  You
could even do it directly in the ELisp file, if point is in the changed
function.  (Read CONTRIBUTE for more details.)

> @@ -308,7 +308,7 @@ 'help-news
>    :supertype 'help-xref
>    'help-function
>    (lambda (file pos)
> -    (pop-to-buffer (find-file-noselect file))
> +    (view-buffer (find-file-noselect file))
>      (goto-char pos))
>    'help-echo (purecopy "mouse-2, RET: show corresponding NEWS announcement"))

This change looks good to me.

Best regards,
Stefan Kangas





reply via email to

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