all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
@ 2020-11-12 13:49 Dario Gjorgjevski
  2020-11-14 16:44 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 9+ messages in thread
From: Dario Gjorgjevski @ 2020-11-12 13:49 UTC (permalink / raw)
  To: 44592

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
  2020-11-12 13:49 bug#44592: In sh-script-mode, should the syntax of . and / be made symbol? Dario Gjorgjevski
@ 2020-11-14 16:44 ` Lars Ingebrigtsen
  2020-12-23 16:05   ` Dario Gjorgjevski
  0 siblings, 1 reply; 9+ messages in thread
From: Lars Ingebrigtsen @ 2020-11-14 16:44 UTC (permalink / raw)
  To: Dario Gjorgjevski; +Cc: 44592

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
  2020-11-14 16:44 ` Lars Ingebrigtsen
@ 2020-12-23 16:05   ` Dario Gjorgjevski
  2020-12-23 22:24     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 9+ messages in thread
From: Dario Gjorgjevski @ 2020-12-23 16:05 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44592

>> 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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
  2020-12-23 16:05   ` Dario Gjorgjevski
@ 2020-12-23 22:24     ` Lars Ingebrigtsen
  2020-12-24  3:59       ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Lars Ingebrigtsen @ 2020-12-23 22:24 UTC (permalink / raw)
  To: Dario Gjorgjevski; +Cc: Stefan Monnier, 44592

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
  2020-12-23 22:24     ` Lars Ingebrigtsen
@ 2020-12-24  3:59       ` Stefan Monnier
  2021-08-27 17:09         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2020-12-24  3:59 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Dario Gjorgjevski, 44592

>> 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






^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
  2020-12-24  3:59       ` Stefan Monnier
@ 2021-08-27 17:09         ` Lars Ingebrigtsen
  2021-08-28  9:44           ` Kévin Le Gouguec
  0 siblings, 1 reply; 9+ messages in thread
From: Lars Ingebrigtsen @ 2021-08-27 17:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Dario Gjorgjevski, 44592

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
  2021-08-27 17:09         ` Lars Ingebrigtsen
@ 2021-08-28  9:44           ` Kévin Le Gouguec
  2021-08-29 18:29             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 9+ messages in thread
From: Kévin Le Gouguec @ 2021-08-28  9:44 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Dario Gjorgjevski, Stefan Monnier, 44592

[-- 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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
  2021-08-28  9:44           ` Kévin Le Gouguec
@ 2021-08-29 18:29             ` Lars Ingebrigtsen
  2021-08-29 21:22               ` Kévin Le Gouguec
  0 siblings, 1 reply; 9+ messages in thread
From: Lars Ingebrigtsen @ 2021-08-29 18:29 UTC (permalink / raw)
  To: Kévin Le Gouguec; +Cc: Dario Gjorgjevski, Stefan Monnier, 44592

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44592: In sh-script-mode, should the syntax of . and / be made symbol?
  2021-08-29 18:29             ` Lars Ingebrigtsen
@ 2021-08-29 21:22               ` Kévin Le Gouguec
  0 siblings, 0 replies; 9+ messages in thread
From: Kévin Le Gouguec @ 2021-08-29 21:22 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Dario Gjorgjevski, Stefan Monnier, 44592

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!





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-29 21:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-12 13:49 bug#44592: In sh-script-mode, should the syntax of . and / be made symbol? Dario Gjorgjevski
2020-11-14 16:44 ` Lars Ingebrigtsen
2020-12-23 16:05   ` Dario Gjorgjevski
2020-12-23 22:24     ` Lars Ingebrigtsen
2020-12-24  3:59       ` Stefan Monnier
2021-08-27 17:09         ` Lars Ingebrigtsen
2021-08-28  9:44           ` Kévin Le Gouguec
2021-08-29 18:29             ` Lars Ingebrigtsen
2021-08-29 21:22               ` Kévin Le Gouguec

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.