bug#53817: 28.0.91; sh-mode indent misses on 'if test; then' when 'then' is on 'if' line

bug#53817: 28.0.91; sh-mode indent misses on 'if test; then' when 'then' is on 'if' line

[Doug Maxey]

Howdy,

Just took 28.0.91 out for a spin, and I like that it seems much more
responsive.

However, lots'o bash gets written here, and one of my idioms is:
<code>
if test;then
   do something
fi
   ^
</code>

In emacs-28, when I type the 'fi', the start column is still indented,
point is placed in column 4.

If 'then' gets moved to the line *after* 'if test',
OR
if a space is inserted between the ';' and 'then', '; then'
<code>
if test; then
</code>
it's all good again and point is set correctly at column 1.

<code>
if test
then
    :
fi
^
</code>

OR

<code>
if test; then
    :
fi
^
</code>

CONFIGURATION NOTES:
The settings for sh-mode all appear to be at the defaults, and a check of
my init code only shows
<lisp>
(setq sh-shell-path "/bin/bash"
      sh-find-file-modifies nil)
</lisp>


On further observation, it seems the smie engine thinks the code which is
legal bash has a syntax error, which would be the missing whitespace
after the ';'.  I can see this by the block start not being highlighted
when at the end of the block (the final 'fi' in the if-else-fi) when the
space is missing after the ';', which "heals" when the space is inserted.

Thanks for looking into this.
++doug

NO CRASH

In GNU Emacs 28.0.91 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 3.24.31, cairo version 1.17.4)
 of 2022-02-05 built on bdbe8fdf7a5d41b7ba009c5aa8e267d7
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 35 (Workstation Edition)

Configured using:
 'configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk3
 --with-gpm=no --with-xwidgets --with-modules --with-harfbuzz
 --with-cairo --with-json build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu CC=gcc 'CFLAGS=-DMAIL_USE_LOCKF -O2
 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches
 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
 -Wp,-D_GLIBCXX_ASSERTIONS
 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection'
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE
XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $EMACSLOADPATH: $HOME/.config/emacs/lisp:$HOME/sb/bats-mode:$HOME/lfin/lisp:/usr/local/lisp/packages:
  value of $LANG: C
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Shell-script

Minor modes in effect:
  magit-auto-revert-mode: t
  flyspell-mode: t
  yas-minor-mode: t
  global-git-commit-mode: t
  shell-dirtrack-mode: t
  dwm-time-keys: t
  which-function-mode: t
  savehist-mode: t
  save-place-mode: t
  global-whitespace-mode: t
  global-auto-revert-mode: t
  tooltip-mode: t
  global-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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Load-path shadows:
/usr/local/lisp/packages/xcscope-1.5/xcscope hides /usr/share/emacs/site-lisp/xcscope
/usr/local/lisp/packages/yaml-mode-0.0.15/yaml-mode hides /usr/share/emacs/site-lisp/yaml-mode/yaml-mode
/usr/local/lisp/packages/transient-0.3.7/transient hides /usr/share/emacs/28.0.91/lisp/transient
$HOME/.config/emacs/lisp/sieve-mode hides /usr/share/emacs/28.0.91/lisp/net/sieve-mode

Features:
(shadow sort mail-extr emacsbug sendmail cus-edit cus-start wid-edit
help-fns radix-tree pcase jka-compr files-x tabify vc-cvs vc-rcs vc
bug-reference magit-submodule magit-obsolete magit-popup magit-blame
magit-stash magit-reflog magit-bisect magit-push magit-pull magit-fetch
magit-clone magit-remote magit-commit magit-sequence magit-notes
magit-worktree magit-tag magit-merge magit-branch magit-reset
magit-files magit-refs magit-status magit magit-repos magit-apply
magit-wip magit-log magit-diff smerge-mode diff magit-core
magit-autorevert magit-margin magit-transient magit-process misearch
multi-isearch vc-git diff-mode vc-dispatcher sh-script smie executable
flyspell ispell yasnippet-snippets yasnippet yaml-mode git-commit
with-editor shell pcomplete server log-edit message rmc puny dired
dired-loaddefs rfc822 mml mml-sec epa epg rfc6068 epg-config gnus-util
rmail rmail-loaddefs time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev
mail-utils gmm-utils mailheader pcvs-util add-log magit-mode transient
cl-extra help-mode format-spec magit-git magit-section magit-utils crm
dash vlc-minor-mode dwm-time-keys edmacro kmacro cperl-mode facemenu
perl-stuff mouse-copy mkwdlist generic local-generic generic-x ffap
thingatpt dwm gud easy-mmode derived cc-mode cc-fonts cc-guess cc-menus
cc-styles cc-align cc-cmds cc-engine cc-vars cc-defs dwm-c-mode
vscode-dark-plus-theme which-func imenu savehist saveplace grep
whitespace autorevert filenotify desktop frameset cus-load clang-rename
clang-include-fixer let-alist clang-format xml rx warnings compile
text-property-search comint ansi-color ring pp info finder-inf package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd 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 cl-generic 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 simple abbrev obarray cl-preloaded nadvice
button loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads xwidget-internal dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 499443 47296)
 (symbols 48 27392 1)
 (strings 32 149217 10619)
 (string-bytes 1 4315046)
 (vectors 16 57071)
 (vector-slots 8 1142242 88614)
 (floats 8 116 295)
 (intervals 56 8513 232)
 (buffers 992 31))

-- 
take care,
++doug
--
*Doug Maxey

bug#53817: 28.0.91; sh-mode indent misses on 'if test; then' when 'then' is on 'if' line

[Lars Ingebrigtsen]

Doug Maxey <emacs-bugs@maxeygroup.tech> writes:

> However, lots'o bash gets written here, and one of my idioms is:
> <code>
> if test;then
>    do something
> fi
>    ^
> </code>

The problem seems to be here:

(defun sh-smie--default-backward-token ()
[...]
            (progn (skip-syntax-backward ".w_'")
                   (or (not (zerop (skip-syntax-backward "\\")))
                       (when (eq ?\\ (char-before (1- (point))))

Semi-colons have punctuation syntax, so we're skipping back over the
entirety of "test;then" instead of stopping after "then".

This looks easy enough to fix, but the overall logic of that function is
somewhat obscure to me, so I've added Stefan to the comments; perhaps he
has some comments.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#53817: 28.0.91; sh-mode indent misses on 'if test; then' when 'then' is on 'if' line

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

> (defun sh-smie--default-backward-token ()
> [...]
>             (progn (skip-syntax-backward ".w_'")
>                    (or (not (zerop (skip-syntax-backward "\\")))
>                        (when (eq ?\\ (char-before (1- (point))))
>
> Semi-colons have punctuation syntax, so we're skipping back over the
> entirety of "test;then" instead of stopping after "then".

We should indeed only skip over "then", and then only over ";" and then
only over "test".

> This looks easy enough to fix, but the overall logic of that function is
> somewhat obscure to me, so I've added Stefan to the comments; perhaps he
> has some comments.

The sh syntax is fairly nasty to parse, especially backwards, so I can't
give you any general comment here.  I'd try something and then see if it
breaks any tests (including looking at test/manual/indent/shell.sh).

For sure, I'd recommend adding a regression test.


        Stefan

bug#53817: 28.0.91; sh-mode indent misses on 'if test; then' when 'then' is on 'if' line

[Doug Maxey]

[-- Attachment #1.1: Type: text/plain, Size: 1202 bytes --]

On Sun, 2022-02-06 at 19:11 -0500, Stefan Monnier wrote:
> > (defun sh-smie--default-backward-token ()
> > [...]
> >             (progn (skip-syntax-backward ".w_'")
> >                    (or (not (zerop (skip-syntax-backward "\\")))
> >                        (when (eq ?\\ (char-before (1- (point))))
> > 
> > Semi-colons have punctuation syntax, so we're skipping back over
> > the
> > entirety of "test;then" instead of stopping after "then".
> 
> We should indeed only skip over "then", and then only over ";" and
> then
> only over "test".
> 
> > This looks easy enough to fix, but the overall logic of that
> > function is
> > somewhat obscure to me, so I've added Stefan to the comments;
> > perhaps he
> > has some comments.
> 
> The sh syntax is fairly nasty to parse, especially backwards, so I
> can't
> give you any general comment here.  I'd try something and then see if
> it
> breaks any tests (including looking at test/manual/indent/shell.sh).
> 
> For sure, I'd recommend adding a regression test.

just a further point, if it helps, this was fine in 27.2.
 
> 
> 
>         Stefan
> 


[-- Attachment #1.2: Type: text/html, Size: 2233 bytes --]

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#53817: 28.0.91; sh-mode indent misses on 'if test; then' when 'then' is on 'if' line

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The sh syntax is fairly nasty to parse, especially backwards, so I can't
> give you any general comment here.  I'd try something and then see if it
> breaks any tests (including looking at test/manual/indent/shell.sh).
>
> For sure, I'd recommend adding a regression test.

I've now tweaked the loop, and it fixes the issue and doesn't seem to
break anything in the manual test tile.  (And I added some automatic
tests.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no