bug#753: [Fwd: sh-script.el: indentation of ( )]

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Daniel Pfeiffer]

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

Hi,

since I abandoned Shell in favour of Perl, over to you maintainers:

-------- Original-Nachricht --------
Betreff: 	sh-script.el: indentation of ( )
Datum: 	Wed, 20 Aug 2008 12:12:12 -0400
Von: 	Darren Stuart Embry <dse@webonastick.com>
An: 	Daniel Pfeiffer <occitan@esperanto.org>



Hello,

I'd like to report an issue regarding indentation of compound commands
surrounded with parenthesis.

Desired indentation:

	#!/bin/sh

	(
	    true
	)
	{
	    true
	}

What indent-region on the entire buffer yields:

	#!/bin/sh
	
	(
	    true
	    )
	{
	    true
	}

Note that the same behavior does not happen with the { } pair.

You have any idea why this is happening and/or how to fix this?

Version of emacs:
	GNU Emacs 22.2.1 (i486-pc-linux-gnu, X toolkit, Xaw3d scroll
	bars) of 2008-04-27 on raven, modified by Debian

Version of sh-script.el:
	;; Author: Daniel Pfeiffer <occitan@esperanto.org>
	;; Version: 2.0f

Thanks in advance.

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in email?



coralament / best Grötens / liebe Grüße / best regards / elkorajn salutojn
Daniel Pfeiffer

-- 
lerne / learn / apprends / lär dig / ucz się    Esperanto:
                    http://lernu.net  /  http://ikurso.net


[-- Attachment #2: Type: text/html, Size: 2525 bytes --]

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Glenn Morris]

Daniel Pfeiffer wrote:

> What indent-region on the entire buffer yields:
>
> 	#!/bin/sh
> 	
> 	(
> 	    true
> 	    )
> 	{
> 	    true
> 	}


Emacs 21.4 gets it right here, whereas 22.1 and later gets it wrong.

The difference seems to be caused by sh-get-kw, specifically:

2006-10-10  Stefan Monnier  <monnier@iro.umontreal.ca>

    * progmodes/sh-script.el (sh-get-kw): | is not among the allowed chars
    for a keyword.

In making this change (rev 1.191), you also added '()' to the
non-allowed characters along with '|'. Since it wasn't documented in
the ChangeLog, was this a mistake?

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Glenn Morris]

Glenn Morris wrote:

> The difference seems to be caused by sh-get-kw, specifically:
>
> 2006-10-10  Stefan Monnier  <monnier@iro.umontreal.ca>
>
>     * progmodes/sh-script.el (sh-get-kw): | is not among the allowed chars
>     for a keyword.
>
> In making this change (rev 1.191), you also added '()' to the
> non-allowed characters along with '|'. Since it wasn't documented in
> the ChangeLog, was this a mistake?

Ping.

Can you remember why you added '()'?

Here's why the '|' got added:

http://lists.gnu.org/archive/html/emacs-pretest-bug/2006-10/msg00160.html

bug#753: marked as done ([Fwd: sh-script.el: indentation of ( )])

[Emacs bug Tracking System]

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


Your message dated Sat, 06 Sep 2008 14:59:20 -0400
with message-id <eqr67xumlz.fsf@fencepost.gnu.org>
and subject line Re: bug#753: [Fwd: sh-script.el: indentation of ( )]
has caused the Emacs bug report #753,
regarding [Fwd: sh-script.el: indentation of ( )]
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
753: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=753
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 6672 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 1421 bytes --]

Hi,

since I abandoned Shell in favour of Perl, over to you maintainers:

-------- Original-Nachricht --------
Betreff: 	sh-script.el: indentation of ( )
Datum: 	Wed, 20 Aug 2008 12:12:12 -0400
Von: 	Darren Stuart Embry <dse@webonastick.com>
An: 	Daniel Pfeiffer <occitan@esperanto.org>



Hello,

I'd like to report an issue regarding indentation of compound commands
surrounded with parenthesis.

Desired indentation:

	#!/bin/sh

	(
	    true
	)
	{
	    true
	}

What indent-region on the entire buffer yields:

	#!/bin/sh
	
	(
	    true
	    )
	{
	    true
	}

Note that the same behavior does not happen with the { } pair.

You have any idea why this is happening and/or how to fix this?

Version of emacs:
	GNU Emacs 22.2.1 (i486-pc-linux-gnu, X toolkit, Xaw3d scroll
	bars) of 2008-04-27 on raven, modified by Debian

Version of sh-script.el:
	;; Author: Daniel Pfeiffer <occitan@esperanto.org>
	;; Version: 2.0f

Thanks in advance.

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in email?



coralament / best Grötens / liebe Grüße / best regards / elkorajn salutojn
Daniel Pfeiffer

-- 
lerne / learn / apprends / lär dig / ucz się    Esperanto:
                    http://lernu.net  /  http://ikurso.net


[-- Attachment #2.1.2: Type: text/html, Size: 2525 bytes --]

[-- Attachment #3: Type: message/rfc822, Size: 1689 bytes --]

From: Glenn Morris <rgm@gnu.org>
To: 753-done@emacsbugs.donarmstrong.com
Subject: Re: bug#753: [Fwd: sh-script.el: indentation of ( )]
Date: Sat, 06 Sep 2008 14:59:20 -0400
Message-ID: <eqr67xumlz.fsf@fencepost.gnu.org>

2008-09-06  Glenn Morris  <rgm@gnu.org>

    * progmodes/sh-script.el (sh-get-kw): Remove '()' from the list of
      unallowed characters; added 2006-10-10 without comment.  (Bug#753)

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Stefan Monnier]

>> The difference seems to be caused by sh-get-kw, specifically:
>> 
>> 2006-10-10  Stefan Monnier  <monnier@iro.umontreal.ca>
>> 
>> * progmodes/sh-script.el (sh-get-kw): | is not among the allowed chars
>> for a keyword.
>> 
>> In making this change (rev 1.191), you also added '()' to the
>> non-allowed characters along with '|'. Since it wasn't documented in
>> the ChangeLog, was this a mistake?

> Ping.

> Can you remember why you added '()'?

Because neither ( nor ) are allowed in keywords.


        Stefan

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Glenn Morris]

[ You must unarchive old bugs before mailing them ]

Stefan Monnier wrote:

>> Can you remember why you added '()'?
>
> Because neither ( nor ) are allowed in keywords.

They _are_ keywords, along with { and }, as evidenced by their entries
in the constant `sh-kw'.

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Stefan Monnier]

> [ You must unarchive old bugs before mailing them ]

Yes: I didn't know it was old :-(

>>> Can you remember why you added '()'?
>> Because neither ( nor ) are allowed in keywords.
> They _are_ keywords, along with { and }, as evidenced by their entries
> in the constant `sh-kw'.

Hmm... I think I see what might be the problem, tho I don't understand
the code enough.  Basically, what I saw is that it recognized "done|" as
a keyword, which I fixed by adding ?| to the "not in keywords" chars.
By the same reasonging "done)" is not a keyword, so I added ?\)
(and ?\( as well for good measure).  I guess that sh-get-kw should be
fixed more robustly by recognizing ")" and "(" but not "(done", nor
"(done|toto)" for that matter.
Can you take care of that and make sure it fixes the problem at hand
without breaking the problem that prompted my misguided fix?


        Stefan

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Glenn Morris]

Stefan Monnier wrote:

> Basically, what I saw is that it recognized "done|" as a keyword,
> which I fixed by adding ?| to the "not in keywords" chars. By the
> same reasonging "done)" is not a keyword, so I added ?\) (and ?\( as
> well for good measure).

Adding them causes the problem you are trying to avoid.

Applying (skip-chars-forward "^ \t\n;&|")

to "done) "

results in "done)", which is not a recognized keyword.

>  I guess that sh-get-kw should be fixed more robustly by recognizing
> ")" and "(" but not "(done", nor "(done|toto)" for that matter. Can
> you take care of that and make sure it fixes the problem at hand
> without breaking the problem that prompted my misguided fix?

It's tricky. Consider the following two examples:


for f in 1; do
    case $f in
        done) echo t ;;
    esac
done


(for f in 1; do
   echo $f
done)


In the second one, "done)" ends a for loop, in the first one it does not.

Both Emacs 22.3 and the current CVS get both examples wrong, in
different ways.

ii), though valid syntax, seems ugly (and hopefully uncommon) to me,
especially considering that in Bash at least, the same problem cannot
arise with {}, since these must be isolated by whitespace.

A simple fix for i) is as follows. I don't know how to fix both i) and ii).


Did you have any other examples of shell syntax to be considered?



*** sh-script.el	5 Jan 2009 03:23:50 -0000	1.220
--- sh-script.el	9 Jan 2009 02:22:31 -0000
***************
*** 1421,1427 ****
    "Make a regexp which matches WORD as a word.
  This specifically excludes an occurrence of WORD followed by
  punctuation characters like '-'."
!   (concat word "\\([^-[:alnum:]_]\\|$\\)"))
  
  (defconst sh-re-done (sh-mkword-regexpr "done"))
  
--- 1421,1428 ----
    "Make a regexp which matches WORD as a word.
  This specifically excludes an occurrence of WORD followed by
  punctuation characters like '-'."
!   ;; ")}" excludes things like "done)" in case statements.
!   (concat word "\\([^-[:alnum:]_)}]\\|$\\)"))
  
  (defconst sh-re-done (sh-mkword-regexpr "done"))

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Stefan Monnier]

>> Basically, what I saw is that it recognized "done|" as a keyword,
>> which I fixed by adding ?| to the "not in keywords" chars. By the
>> same reasonging "done)" is not a keyword, so I added ?\) (and ?\( as
>> well for good measure).

> Adding them causes the problem you are trying to avoid.

I don't follow you.

> Applying (skip-chars-forward "^ \t\n;&|")
> to "done) "
> results in "done)", which is not a recognized keyword.

So does the original code (skip-chars-forward "^ \t\n;&"), but so
doesn't my code (skip-chars-forward "^ \t\n;&|()").  In this sense my
code did make things more correct.

>> I guess that sh-get-kw should be fixed more robustly by recognizing
>> ")" and "(" but not "(done", nor "(done|toto)" for that matter. Can
>> you take care of that and make sure it fixes the problem at hand
>> without breaking the problem that prompted my misguided fix?

> It's tricky. Consider the following two examples:

> for f in 1; do
>     case $f in
>         done) echo t ;;
>     esac
> done


> (for f in 1; do
>    echo $f
> done)

> In the second one, "done)" ends a for loop, in the first one it does not.

It doesn't matter.  "done)" is not an entity in `sh', so we should not
treat it as one.  After all if you replace "done)" with "done )" in the
above 2 scripts, they still mean the same.

> Both Emacs 22.3 and the current CVS get both examples wrong, in
> different ways.

Yes, but it's a due to different bug than the one at hand, IIUC.


        Stefan

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Glenn Morris]

Stefan Monnier wrote:

> It doesn't matter.  "done)" is not an entity in `sh', so we should not
> treat it as one.

Yes, but it is a useful shortcut to treat it as one IMO, since it
gets the indentation "mostly right" for little effort.

I'm afraid I don't know how to fix it (indentation) properly, so I
reopened this bug.

>  After all if you replace "done)" with "done )" in the above 2
> scripts, they still mean the same.

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Stefan Monnier]

>> It doesn't matter.  "done)" is not an entity in `sh', so we should not
>> treat it as one.
> Yes, but it is a useful shortcut to treat it as one IMO, since it
> gets the indentation "mostly right" for little effort.

It still gets it wrong for "done )".  And it introduces bugs elsewhere.
I'm pretty sure it was not done on purpose: it's basically a bug that
accidentally does the right thing in some corner cases.

> I'm afraid I don't know how to fix it (indentation) properly, so I
> reopened this bug.

We know how to fix the OP's bug without re-introducing the bug that my
misguided patch tried to fix.  So we should do that and close this bug.
Then we can open another bug about the "done )" case pattern since it's
a different bug.


        Stefan

bug#753: marked as done ([Fwd: sh-script.el: indentation of ( )])

[Emacs bug Tracking System]

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


Your message dated Mon, 04 May 2009 20:24:50 -0900
with message-id <27J1829938287LEVid3xyt@demarken.nl>
and subject line African Tribe Secrets That Will Shock You: The Masaii And White Women Tourists
has caused the Emacs bug report #753,
regarding [Fwd: sh-script.el: indentation of ( )]
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
753: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=753
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 6672 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 1421 bytes --]

Hi,

since I abandoned Shell in favour of Perl, over to you maintainers:

-------- Original-Nachricht --------
Betreff: 	sh-script.el: indentation of ( )
Datum: 	Wed, 20 Aug 2008 12:12:12 -0400
Von: 	Darren Stuart Embry <dse@webonastick.com>
An: 	Daniel Pfeiffer <occitan@esperanto.org>



Hello,

I'd like to report an issue regarding indentation of compound commands
surrounded with parenthesis.

Desired indentation:

	#!/bin/sh

	(
	    true
	)
	{
	    true
	}

What indent-region on the entire buffer yields:

	#!/bin/sh
	
	(
	    true
	    )
	{
	    true
	}

Note that the same behavior does not happen with the { } pair.

You have any idea why this is happening and/or how to fix this?

Version of emacs:
	GNU Emacs 22.2.1 (i486-pc-linux-gnu, X toolkit, Xaw3d scroll
	bars) of 2008-04-27 on raven, modified by Debian

Version of sh-script.el:
	;; Author: Daniel Pfeiffer <occitan@esperanto.org>
	;; Version: 2.0f

Thanks in advance.

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in email?



coralament / best Grötens / liebe Grüße / best regards / elkorajn salutojn
Daniel Pfeiffer

-- 
lerne / learn / apprends / lär dig / ucz się    Esperanto:
                    http://lernu.net  /  http://ikurso.net


[-- Attachment #2.1.2: Type: text/html, Size: 2525 bytes --]

[-- Attachment #3: Type: message/rfc822, Size: 16679 bytes --]

[-- Attachment #3.1.1: Type: image/png, Size: 11461 bytes --]

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Andrew Hyatt]

+occitan (the original reported)

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> It doesn't matter.  "done)" is not an entity in `sh', so we should not
>>> treat it as one.
>> Yes, but it is a useful shortcut to treat it as one IMO, since it
>> gets the indentation "mostly right" for little effort.
>
> It still gets it wrong for "done )".  And it introduces bugs elsewhere.
> I'm pretty sure it was not done on purpose: it's basically a bug that
> accidentally does the right thing in some corner cases.
>
>> I'm afraid I don't know how to fix it (indentation) properly, so I
>> reopened this bug.
>
> We know how to fix the OP's bug without re-introducing the bug that my
> misguided patch tried to fix.  So we should do that and close this bug.
> Then we can open another bug about the "done )" case pattern since it's
> a different bug.
>
>
>         Stefan

I can't reproduce the original bug on Emacs 25.  Can anyone reproduce
it?  If not, or I don't hear anything in a few weeks, I'll close this.

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Glenn Morris]

Andrew Hyatt wrote:

> I can't reproduce the original bug on Emacs 25.  Can anyone reproduce
> it?  If not, or I don't hear anything in a few weeks, I'll close this.

The remaining issue was not with the original recipe, it was with the
related stuff that was discussed later. You might like to check if any
of it is still relevant.

bug#753: [Fwd: sh-script.el: indentation of ( )]

[Andrew Hyatt]

Glenn Morris <rgm@gnu.org> writes:

> Andrew Hyatt wrote:
>
>> I can't reproduce the original bug on Emacs 25.  Can anyone reproduce
>> it?  If not, or I don't hear anything in a few weeks, I'll close this.
>
> The remaining issue was not with the original recipe, it was with the
> related stuff that was discussed later. You might like to check if any
> of it is still relevant.

There is a problem, but it seems fairly minor, and arguably not a bug -
if you use a case identifier that's also a keyword, it doesn't behave
correctly.  That has a trivial workaround - just wrap the keyword in
quotes.  I think to some extent, modes having issues with keywords
acting as identifiers or variables is pretty normal - at least hard to
fix without real semantic parsing.

I'm going to close this bug, and if anyone feels like there's still an
issue worth fixing here, please open a new bug.

Thanks for the response!