On Fri, 11 Apr 2025 11:57:39 +0300 Eli Zaretskii wrote: >> From: Stephen Berman >> Cc: Eli Zaretskii , Arash Esbati , Stefan >> Monnier , 77118@debbugs.gnu.org >> Date: Fri, 11 Apr 2025 10:19:35 +0200 >> >> On Thu, 20 Mar 2025 08:57:52 +0100 Stephen Berman wrote: >> >> > On Wed, 19 Mar 2025 12:15:40 -0700 Stefan Kangas wrote: >> > >> >> Eli Zaretskii writes: >> >> >> >>>> From: Arash Esbati >> >>>> Date: Wed, 19 Mar 2025 14:05:21 +0100 >> >>>> >> >>>> when I eval the following code: >> >>>> >> >>>> --8<---------------cut here---------------start------------->8--- >> >>>> (let ((help-form (substitute-command-keys "\ >> >>>> Select with a key: >> >>>> \\`h' do this, >> >>>> \\`k' do that, >> >>>> \\`RET' do nothing."))) >> >>>> (read-char-choice (substitute-command-keys >> >>>> (format "\ >> >>>> This (\\`h'), That (\\`k'), Nothing (\\`RET'), Help (\\`%s'): " >> >>>> (key-description (vector help-char)))) >> >>>> '(?h ?k ?\r))) >> >>>> --8<---------------cut here---------------end--------------->8--- >> >>>> >> >>>> and do 'C-h', h, k and RET don't receive the help-key-binding face in >> >>>> the help buffer which pops up. What I see with 'emacs -Q' is attached. >> >>>> Note the difference in the minibuffer. >> >>>> >> >>>> The reason seems to be in the implementation of `help-form-show' using >> >>>> `princ' which strips properties. If this is intentional, it doesn't >> >>>> match definitions of `help-form' in Emacs tree. Take for example >> >>>> dired-aux.el which pushes the string for `help-form' also through >> >>>> `substitute-command-keys'[1,2,3]. >> >>>> >> >>>> In case this report is confirmed, a possible solution is provided >> >>>> here[4]. >> >>> >> >>> Stefan, any comments or reasons why we use princ there? >> >> >> >> There is no deep reason. We want to stay in the buffer that we invoked >> >> help from, to get the keybindings from the right keymaps, but we also >> >> want to insert the documentation in the *Help* buffer. Setting >> >> `standard-output` to the *Help* buffer and using `princ` is just a >> >> convenient way to do that. >> >> >> >> The fix is to switch things around so we can use `insert` instead. >> >> We have done that elsewhere in help, and should do the same in >> >> `read-char-choice` also. >> > >> > Is the patch I posted to help-gnu-emacs -- linked in the OP of this bug >> > thread and repeated below for convenience -- adequate? >> > >> > Steve Berman >> > >> > diff --git a/lisp/help.el b/lisp/help.el >> > index 835e47fec43..d77894b3372 100644 >> > --- a/lisp/help.el >> > +++ b/lisp/help.el >> > @@ -2190,8 +2190,8 @@ help-form-show >> > "Display the output of a non-nil `help-form'." >> > (let ((msg (eval help-form t))) >> > (if (stringp msg) >> > - (with-output-to-temp-buffer " *Char Help*" >> > - (princ msg))))) >> > + (with-help-window " *Char Help*" >> > + (insert msg))))) >> > >> > (defun help--append-keystrokes-help (str) >> > (let* ((keys (this-single-command-keys)) >> >> This bug thread seems to have stalled; is my patch acceptable or is >> someone working on the alternative suggested upthread by Stefan Kangas >> (a version of princ that preserves string properties)? If my patch is >> acceptable, should it be installed in emacs-30 or master? > > TBH, this patch scares me, even if we only install it on master, let > alone on the release branch. with-help-window's implementation has > nothing in common with that of with-output-to-temp-buffer, so who > knows what unintended changes in behavior this deceptively-simple > change will cause? E.g., with-help-window obeys the value of > help-window-select wrt selecting the window: do we want that here? > And that's just the tip of what I fear is a very large iceberg. All > that just to support some obscure use case? > > Can't we come up with some change that has more predictable > consequences? Like some creative use of with-current-buffer around > the call to 'insert'? That'd have only local effect, which is well > understood, and thus make this change easier to agree to and install > on the release branch. Is the patch below creative enough? Steve Berman