bug#62453: 30.0.50; Semicolon in view-lossage yields wrong alignment

bug#62453: 30.0.50; Semicolon in view-lossage yields wrong alignment

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

It seems like key sequences that end with a semicolon hinder the
alignment in the `view-lossage` output buffer.  This is a regression
with respect to the (IMO correct) behavior in Emacs 27.

With Emacs from master:
1. emacs -Q
2. type C-;
3. type ;
4. type C-h l
5. The *Help* buffer now shows:

 C-			; ;; nil
 ; ;; self-insert-command
 C-h l ;; view-lossage

Note that all three lines are formatted incorrectly compared to what we
get with Emacs 27:

 C-;			;; nil
 ;			;; self-insert-command
 C-h l			;; view-lossage

Thanks,
Eshel



In GNU Emacs 30.0.50 (build 29, x86_64-apple-darwin22.3.0, NS
 appkit-2299.40 Version 13.2.1 (Build 22D68)) of 2023-03-25 built on
 esmac
Repository revision: db7e95531ac36ae842787b6c5f2859d0642c78cc
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2299
System Description:  macOS 13.2.1

Configured using:
 'configure CFLAGS=-O3 --with-native-compilation --with-json
 --with-tree-sitter'

Configured features:
ACL DBUS GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
NOTIFY KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM ZLIB

Important settings:
  value of $LC_CTYPE: UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date cl-loaddefs
comp comp-cstr warnings icons subr-x rx cl-seq cl-macs gv cl-extra
help-mode bytecomp byte-compile cl-lib rmc iso-transl tooltip cconv
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/ns-win ns-win ucs-normalize mule-util
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind kqueue cocoa ns lcms2 multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 78358 8406)
 (symbols 48 7093 0)
 (strings 32 19420 1908)
 (string-bytes 1 594120)
 (vectors 16 15982)
 (vector-slots 8 279688 11110)
 (floats 8 30 46)
 (intervals 56 223 0)
 (buffers 984 12))

bug#62453: 30.0.50; Semicolon in view-lossage yields wrong alignment

[Eli Zaretskii]

> Date: Sun, 26 Mar 2023 08:33:18 +0300
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> It seems like key sequences that end with a semicolon hinder the
> alignment in the `view-lossage` output buffer.  This is a regression
> with respect to the (IMO correct) behavior in Emacs 27.
> 
> With Emacs from master:
> 1. emacs -Q
> 2. type C-;
> 3. type ;
> 4. type C-h l
> 5. The *Help* buffer now shows:
> 
>  C-			; ;; nil
>  ; ;; self-insert-command
>  C-h l ;; view-lossage
> 
> Note that all three lines are formatted incorrectly compared to what we
> get with Emacs 27:
> 
>  C-;			;; nil
>  ;			;; self-insert-command
>  C-h l			;; view-lossage

Does the patch below give good results?

Stefan, any better ideas?  It looks like relying on comment-indent is
quite fragile here, as keys can include a semi-colon, which will be
interpreted as beginning a comment in too many places.  I wonder
whether the two extra bindings I add below really plug all the holes.

diff --git a/lisp/help.el b/lisp/help.el
index 3e94b50..13e61c0 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -689,6 +689,10 @@ view-lossage
       (with-current-buffer standard-output
 	(goto-char (point-min))
 	(let ((comment-start ";; ")
+              ;; Prevent 'comment-indent' from handling a single
+              ;; semicolon as the beginning of a comment.
+              (comment-start-skip ";; ")
+              (comment-use-syntax nil)
               (comment-column 24))
           (while (not (eobp))
             (comment-indent)

bug#62453: 30.0.50; Semicolon in view-lossage yields wrong alignment

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 26 Mar 2023 08:33:18 +0300
>> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> It seems like key sequences that end with a semicolon hinder the
>> alignment in the `view-lossage` output buffer.  This is a regression
>> with respect to the (IMO correct) behavior in Emacs 27.
>> 
>> [...]
>
> Does the patch below give good results?

Thanks, that works well AFAICT from a few experiments.

> Stefan, any better ideas?  It looks like relying on comment-indent is
> quite fragile here, as keys can include a semi-colon, which will be
> interpreted as beginning a comment in too many places.  I wonder
> whether the two extra bindings I add below really plug all the holes.
>
> diff --git a/lisp/help.el b/lisp/help.el
> index 3e94b50..13e61c0 100644
> --- a/lisp/help.el
> +++ b/lisp/help.el
> @@ -689,6 +689,10 @@ view-lossage
>        (with-current-buffer standard-output
>  	(goto-char (point-min))
>  	(let ((comment-start ";; ")
> +              ;; Prevent 'comment-indent' from handling a single
> +              ;; semicolon as the beginning of a comment.
> +              (comment-start-skip ";; ")
> +              (comment-use-syntax nil)
>                (comment-column 24))
>            (while (not (eobp))
>              (comment-indent)

bug#62453: 30.0.50; Semicolon in view-lossage yields wrong alignment

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Stefan, any better ideas?  It looks like relying on comment-indent is
> quite fragile here,

Agreed.

> as keys can include a semi-colon, which will be
> interpreted as beginning a comment in too many places.  I wonder
> whether the two extra bindings I add below really plug all the holes.

Here's another problem:

     C-x 5 2		 ;; make-frame-command
     ;; handle-switch-frame
     <help-echo> <help-echo> ;; handle-switch-frame
     ;; handle-switch-frame
     ;; handle-switch-frame
     ;; handle-switch-frame
     ;; handle-switch-frame
     ;; handle-switch-frame
     ;; handle-switch-frame
     C-			; ;; nil
     ; ;; self-insert-command
     C-h l ;; view-lossage

I'm not sure why there's no `switch-frame` event but it absence leads to
incorrectly indenting the `;; handle-switch-frame`.

We're probably better off indenting "by hand".


        Stefan

bug#62453: 30.0.50; Semicolon in view-lossage yields wrong alignment

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eshel Yaron <me@eshelyaron.com>,  62453@debbugs.gnu.org
> Date: Sun, 26 Mar 2023 11:11:06 -0400
> 
> We're probably better off indenting "by hand".

That'd be my conclusion as well.  But that's definitely not for the
release branch.

Come to think of that, why not use the :align-to display spec to align
the command descriptions?

bug#62453: 30.0.50; Semicolon in view-lossage yields wrong alignment

[Juri Linkov]

>> We're probably better off indenting "by hand".
>
> That'd be my conclusion as well.  But that's definitely not for the
> release branch.
>
> Come to think of that, why not use the :align-to display spec to align
> the command descriptions?

This format was intended to be compatible with `edit-kbd-macro`,
and it uses such logic in `edmacro-format-keys` for alignment:

		  (make-string (max (- 3 (/ (length desc) 8)) 1) 9)
		  ";; " (if (stringp bind) bind (symbol-name bind))

bug#62453: 30.0.50; Semicolon in view-lossage yields wrong alignment

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eshel Yaron <me@eshelyaron.com>,  62453@debbugs.gnu.org
> Date: Sun, 26 Mar 2023 11:11:06 -0400
> 
> Here's another problem:
> 
>      C-x 5 2		 ;; make-frame-command
>      ;; handle-switch-frame
>      <help-echo> <help-echo> ;; handle-switch-frame
>      ;; handle-switch-frame
>      ;; handle-switch-frame
>      ;; handle-switch-frame
>      ;; handle-switch-frame
>      ;; handle-switch-frame
>      ;; handle-switch-frame
>      C-			; ;; nil
>      ; ;; self-insert-command
>      C-h l ;; view-lossage
> 
> I'm not sure why there's no `switch-frame` event but it absence leads to
> incorrectly indenting the `;; handle-switch-frame`.

Yes, but this is not a regression: Emacs 27 behaves the same.

So I've now installed my "band-aid" fix on the emacs-29 branch.

> We're probably better off indenting "by hand".

I will try to come up with a better fix for master, and will also try
to stay consistent with edmacro-format-keys while at that.