bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Dario Gjorgjevski]

In the syntax table of sh-script-mode, the syntax of . and / is
punctuation.  This has the advantage of being able to use, e.g.,
kill-sexp to kill one component of a path.  However, it also has the
disadvantage of not correctly indenting line continuations when a path
to a command is given.

As an example, consider:

    relative-path/to/configure --prefix=$prefix             \
                               --with-x                     \
                               --with-x-toolkit=gtk3        \
                               --with-cairo

(This is how I would expect it to be indented.)  Currently,
sh-script-mode would consider only relative-path to be the first sexp on
the leading line and therefore indent it as:

    relative-path/to/configure --prefix=$prefix     \
                 --with-x                           \
                 --with-x-toolkit=gtk3              \
                 --with-cairo

Changing the syntax of . and / to symbol would make the entire
relative-path/to/configure be considered the first sexp and therefore
give the correct indentation show above.

Moreover, one would then be able to use, e.g., kill-sexp to kill a full
path as opposed to just one component.

What do you think?  Alternatively, we could work on fixing only the
SMIE-provided indentation and leave the syntax as it is.

Best regards,
Dario

-- 
$ keyserver=hkps://hkps.pool.sks-keyservers.net
$ keyid=744A4F0B4F1C9371
$ gpg --keyserver $keyserver --search-keys $keyid

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Lars Ingebrigtsen]

Dario Gjorgjevski <dario.gjorgjevski@gmail.com> writes:

> Changing the syntax of . and / to symbol would make the entire
> relative-path/to/configure be considered the first sexp and therefore
> give the correct indentation show above.
>
> Moreover, one would then be able to use, e.g., kill-sexp to kill a full
> path as opposed to just one component.

I think that sounds like a too drastic change in how the mode works,
perhaps -- how the killing commands (etc) works gets into your muscle
memory.

> What do you think?  Alternatively, we could work on fixing only the
> SMIE-provided indentation and leave the syntax as it is.

I think that sounds like a better solution.

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

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Dario Gjorgjevski]

>> What do you think?  Alternatively, we could work on fixing only the
>> SMIE-provided indentation and leave the syntax as it is.
>
> I think that sounds like a better solution.

The simplest solution I can think of is to change the

    (skip-syntax-backward "w_'")

in ‘sh-smie--default-backward-token’ to

    (skip-syntax-backward ".w_'").

This fixes the issue, but I’m not sure about unwanted effects.
Any ideas if it could cause something to misbehave?

Best regards,
Dario

-- 
$ keyserver=hkps://hkps.pool.sks-keyservers.net
$ keyid=744A4F0B4F1C9371
$ gpg --keyserver $keyserver --search-keys $keyid

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Lars Ingebrigtsen]

Dario Gjorgjevski <dario.gjorgjevski@gmail.com> writes:

>>> What do you think?  Alternatively, we could work on fixing only the
>>> SMIE-provided indentation and leave the syntax as it is.
>>
>> I think that sounds like a better solution.
>
> The simplest solution I can think of is to change the
>
>     (skip-syntax-backward "w_'")
>
> in ‘sh-smie--default-backward-token’ to
>
>     (skip-syntax-backward ".w_'").
>
> This fixes the issue, but I’m not sure about unwanted effects.
> Any ideas if it could cause something to misbehave?

Perhaps Stefan has some comments here; added to the CCs.

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

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Stefan Monnier]

>> The simplest solution I can think of is to change the
>>
>>     (skip-syntax-backward "w_'")
>>
>> in ‘sh-smie--default-backward-token’ to
>>
>>     (skip-syntax-backward ".w_'").
>>
>> This fixes the issue, but I’m not sure about unwanted effects.
>> Any ideas if it could cause something to misbehave?
>
> Perhaps Stefan has some comments here; added to the CCs.

I can't remember enough of how the syntax tables of sh-mode are setup.
So I suggest you just try it and see how it fares.

W.r.t only tweaking the SMIE behavior vs affecting `kill-sexp`, I think we
have `kill-word` for smaller chunks, so I don't see a convincing need to
use `kill-sexp` to stop at `/` boundaries.

But please add regression tests when you do that (and when you find
that it introduces new problems, please add those cases as additional
tests).


        Stefan

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Lars Ingebrigtsen]

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

>>> The simplest solution I can think of is to change the
>>>
>>>     (skip-syntax-backward "w_'")
>>>
>>> in ‘sh-smie--default-backward-token’ to
>>>
>>>     (skip-syntax-backward ".w_'").
>>>
>>> This fixes the issue, but I’m not sure about unwanted effects.
>>> Any ideas if it could cause something to misbehave?
>>
>> Perhaps Stefan has some comments here; added to the CCs.
>
> I can't remember enough of how the syntax tables of sh-mode are setup.
> So I suggest you just try it and see how it fares.
>
> W.r.t only tweaking the SMIE behavior vs affecting `kill-sexp`, I think we
> have `kill-word` for smaller chunks, so I don't see a convincing need to
> use `kill-sexp` to stop at `/` boundaries.
>
> But please add regression tests when you do that (and when you find
> that it introduces new problems, please add those cases as additional
> tests).

I've now applied Dario's change to Emacs 28 (and added a test case).

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

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Kévin Le Gouguec]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I've now applied Dario's change to Emacs 28 (and added a test case).

I wonder, would it make sense to also add ){ and (} to handle cases with
${braces}?  E.g. I'd like this indentation:

${path_to_root}/configure --prefix=$prefix\
                          --with-x

but right now (55e77a811) I am getting this:

${path_to_root}/configure --prefix=$prefix\
               --with-x

I'm don't know all that much about syntax classes and SMIE; still,
here's a "monkey see; monkey do" patch that works for me:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: further.patch --]
[-- Type: text/x-patch, Size: 1468 bytes --]

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 201d1fd164..2d095c2a13 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1775,7 +1775,7 @@ sh-smie--default-backward-token
                      (goto-char p)
                      nil))))
         (while
-            (progn (skip-syntax-backward ".w_'")
+            (progn (skip-syntax-backward ".w_'){(}")
                    (or (not (zerop (skip-syntax-backward "\\")))
                        (when (eq ?\\ (char-before (1- (point))))
                          (let ((p (point)))
diff --git a/test/lisp/progmodes/sh-script-tests.el b/test/lisp/progmodes/sh-script-tests.el
index 5bdce6260a..82c2d5168c 100644
--- a/test/lisp/progmodes/sh-script-tests.el
+++ b/test/lisp/progmodes/sh-script-tests.el
@@ -35,6 +35,17 @@ test-sh-script-indentation
     (should (equal
              (buffer-substring-no-properties (point-min) (point-max))
              "relative-path/to/configure --prefix=$prefix\\
-			   --with-x"))))
+			   --with-x")))
+  (with-temp-buffer
+    (insert "${path_to_root}/configure --prefix=$prefix\\
+             --with-x")
+    (shell-script-mode)
+    (goto-char (point-min))
+    (forward-line 1)
+    (indent-for-tab-command)
+    (should (equal
+             (buffer-substring-no-properties (point-min) (point-max))
+             "${path_to_root}/configure --prefix=$prefix\\
+			  --with-x"))))
 
 ;;; sh-script-tests.el ends here

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Lars Ingebrigtsen]

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> -            (progn (skip-syntax-backward ".w_'")
> +            (progn (skip-syntax-backward ".w_'){(}")

I don't think that's quite correct -- { and } aren't syntax classes.
But just adding ( and ) seems to make the test case indent correctly, so
I've now pushed that to the trunk.

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

bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?

[Kévin Le Gouguec]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:
>
>> -            (progn (skip-syntax-backward ".w_'")
>> +            (progn (skip-syntax-backward ".w_'){(}")
>
> I don't think that's quite correct -- { and } aren't syntax classes.
> But just adding ( and ) seems to make the test case indent correctly, so
> I've now pushed that to the trunk.

Ah, right!  I had naively used C-u C-x = to check what syntax classes {
and } belonged to, and that told me "(}" and "){", respectively.

Thanks for your amendment!