skeleton.el machinery eats newlines.

skeleton.el machinery eats newlines.

[Luc Teirlinck]

Would it be possible for somebody more familiar with the skeleton.el
machinery than I am to take a look at the following?  Something in the
skeleton.el machinery seems to "eat newlines".

The actual manifestations of the problem (at least the ones known to
me) occur in the sh-mode commands sh-for and sh-case, but I believe
that some bug in skeleton.el is the actual underlying cause.

The commands sh-for and sh-case malfunction in the current CVS Emacs
and have apparently been malfunctioning for some time.  I used them in
Emacs20.7, but somehow I have not been using those commands again
until very recently.  The problem becomes immediately apparent by
putting a buffer in sh-mode for bash (other shells probably give
similar problems, but bash is what I use and what I looked at) and
then using C-c TAB and C-c C-c, answering, say "aa" , or whatever, to
all minibuffer questions.  Do this first in Emacs20.7 and then in
current CVS emacs.  Results:

Emacs20.7:

if [ aa ]; then
    
elif [ aa ]; then
    
elif [ aa ]; then
    
elif [ aa ]; then
    
else
    
fi

case aa in
    aa)
	
	;;
    aa)
	
	;;
    aa)
	
	;;
    aa)
	
	;;
    aa)
	
	;;
    aa)
	
	;;
    *)
	
esac

Looks perfect (at least to me).

Now today's CVS:

if [ aa ]; then
    
elif [ aa ]; then
elif [ aa ]; then
elif [ aa ]; then
elif [ aa ]; then
else
    
fi

case aa in
    aa)
    
    ;;
    aa)
    
    ;;aa)
    
    ;;aa)
    
    ;;aa)
    
	;;*)
	
esac


Problem: Some obvious newlines are missing.  So, at first we guess
that there are newlines missing in the define-skeleton 's for those
commands.  But no, all necessary newlines seem to be there.  It is
actually perfectly possible to revert to the Emacs20.7 behavior by
adding extra newlines anyway.  The elif forms require two newlines in
the buffer.  There are two newlines in the define-skeleton, but we
need three in there to get two in the buffer.  For the case we need
three, there are three newlines in the define-skeleton but four are
needed to get three in the buffer.  

Can somebody familiar with skeleton.el tell me whether for some reason
this is actually expected behavior?  If it is, then the bug is in the
two define-skeleton's and is trivial to correct.  If not, the bug is in
skeleton.el and might be more complex.

A potential candidate for the trouble spot might be changes made by
Stefan, more than 7 months ago, in skeleton-internal-1, involving quite
some games with newlines.  I am absolutely not certain those changes
are the culprit, however.

I believe it would be a lot easier for somebody really familiar with
skeleton.el to get to the bottom of this than it would be for me.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Just to avoid two people investigating the bugs I reported
independently:

I got all my courage together and decided to investigate these bugs
myself.  It actually seems to involve three bugs, one in
skeleton-internal-1 (in skeleton.el), one in sh-get-indent-info (in
sh-script.el) and probably another bug in sh-script.el related to
shell-indentation which I am still hunting down.  I believe I fixed
the first two.

Now sh-if seems to work better than in Emacs20.7 (it actually had a
small bug there, which somebody else appears to have eliminated) and
the newline problem in sh-case is gone too, but the indentation in
sh-case seems still imperfect.  The bug I already fixed in
sh-get-indent-info could produce mysterious beginning of buffer errors
while indenting in sh-mode, not only in sh-case.

Once I get sh-case working the way I want it to, I will still need to
make sure I did not introduce new bugs by fixing those three.  I do
not feel comfortable with all these skeletons.  And it is not even
Halloween!

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

I am not sure any more that the third indentation "bug" I referred to
in my previous posting is not merely a product of my imagination.
Putting a buffer in sh-mode for bash and answering aa to all
minibuffer questions yields:

Current CVS:

case aa in
    aa)
    
    ;;
    aa)
    
    ;;aa)
    
    ;;aa)
    
    ;;aa)
    
	;;*)
	
esac


With my current patch:

case aa in
    aa)
    
    ;;
    aa)
    
    ;;
    aa)
    
    ;;
    aa)
    
    ;;
    *)
    
esac

In Emacs20.7:

case aa in
    aa)
	
	;;
    aa)
	
	;;
    aa)
	
	;;
    aa)
	
	;;
    *)
	
esac

It seems to me that the "shift to the right" of the ;; between
Emacs20.7 and Emacs21 was a deliberate change and not an inadvertent
bug.  I am not an expert in shell indentation.  I am not used to worry
a lot about indentation because I use Emacs and I tend to just use
whichever indentation style Emacs uses.  Some of the shell scripts in
the bash2.05b sources seem closer to the Emacs21 style than the
Emacs20.7 indentation.

I hope that I am not inadvertently starting an ideological "Indentation
War" here.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

I forgot to mention in my previous posting that everything refers to
C-c C-c output (in sh-mode), that point on the blank lines is indented
just like the ;; (in all three cases) and that current CVS is going to
give a "Beginning of buffer" error instead of printing the esac.  The
esac occurs after applying the patch to the "Beginning of buffer"
problem, which is unrelated to the skeleton.el newline problem.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Richard Stallman]

    It seems to me that the "shift to the right" of the ;; between
    Emacs20.7 and Emacs21 was a deliberate change and not an inadvertent
    bug.

It seems like a bug to me, because I agree with you that the result
looks bad.  However, I don't know whether the change was intentional.
If it looks intentional, who made it?

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Richard Stallman wrote:

   It seems like a bug to me, because I agree with you that the result
   looks bad.  However, I don't know whether the change was intentional.
   If it looks intentional, who made it?

The reason I thought it was intentional was because it already appears
in the first few lines where the newline bug does not occur.  But I
now start to believe again that it actually is a third unrelated
indentation bug.  Unless somebody says it was intentional and explains
why the change was made, I will try to revert things to the Emacs20.7
behavior.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Stefan Monnier]

>     It seems to me that the "shift to the right" of the ;; between
>     Emacs20.7 and Emacs21 was a deliberate change and not an inadvertent
>     bug.
> 
> It seems like a bug to me, because I agree with you that the result
> looks bad.  However, I don't know whether the change was intentional.
> If it looks intentional, who made it?

I believe it is because of the following:
in Emacs-20, sh-script did not offer auto-indentation so skeletons used
a "rigid" form of indentation defined in the skeleton's definition.
In Emacs-21, OTOH sh-script offers auto-indentation and skeletons thus
simply rely on it to do their job.

Now why was the indentation style chosen differently in the indentation
code than what the skeletons used, I don't know.


	Stefan

Re: skeleton.el machinery eats newlines.

[Stefan Monnier]

> Would it be possible for somebody more familiar with the skeleton.el
> machinery than I am to take a look at the following?  Something in the
> skeleton.el machinery seems to "eat newlines".

I change skeleton.el to only add starting-\n and ending-\n when
"needed" but it looks like it doesn't do what it should in your case.
I'll take a look at it,


	Stefan

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Stefan Monnier wrote:

   I change skeleton.el to only add starting-\n and ending-\n when
   "needed" but it looks like it doesn't do what it should in your case.
   I'll take a look at it,

Thanks.  As I previously mentioned, I was able to revert to the
previous situation, but in a way that made me feel very uncomfortable
about introducing new bugs elsewhere.  In particular, since I probably
just reintroduced all newlines you deleted, my "fix" would very
probably reintroduce whichever bugs your change was meant to fix.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

The bug in sh-get-indent-info I mentioned earlier in this thread may
have been a "fake", depending on the answer to the question below.

It only occurred because I was experimenting with C-c C-c
without a #!-line.  Hence the code thought it was right after the
#!-line, while it was actually at the beginning of the buffer.

Can we just assume that people NEVER use (or never should use)
shell-scripts without a #!-line?  If one just writes a shell-script
for personal use and only plans to use it from a specific shell the
#!-line is strictly speaking not necessary.  Actually, bash even has a
strange, apparently little known, feature: if you do not put in a
#!-line, the subshell executing the script remembers hashed pathnames.
With the #!-line it does not.  I personally always have a #!-line,
except when I inadvertently forget it or erase it, as I did in this
case.

So can we just assume that there must be a #!-line, in which case
there is no bug in sh-indent-info, or not?

Sincerely,

Luc Teirlinck.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

>From my previous message:

   So can we just assume that there must be a #!-line, in which case
   there is no bug in sh-indent-info, or not?

Meant if `sh-get-indent-info'.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

>From my previous message:

   if you do not put in a #!-line, the subshell executing the script
   remembers hashed pathnames.  With the #!-line it does not.

What I meant was: without a #!-line the subshell remembers the
information stored in the parent's hash table, with a #!-line it does
not.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Glenn Morris]

Luc Teirlinck wrote:

> Can we just assume that people NEVER use (or never should use)
> shell-scripts without a #!-line?

Some shell scripts are meant to be sourced, not executed, and therefore
don't need (and shouldn't have, IMO) the #! line. The obvious ones are
profile files, such as ~/.bashrc, etc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Glenn Morris wrote:

   Luc Teirlinck wrote:

   > Can we just assume that people NEVER use (or never should use)
   > shell-scripts without a #!-line?

   Some shell scripts are meant to be sourced, not executed, and therefore
   don't need (and shouldn't have, IMO) the #! line. The obvious ones are
   profile files, such as ~/.bashrc, etc.

Of course, you are right.  I forgot the obvious for a moment.
I will send my candidate fix for the bug somewhat later.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Here is the description of the bug in sh-get-indent-info I have been
referring to (the description refers to the current CVS):

Start with the following buffer in sh-mode.  Make sure that the `case'
is at the beginning of the buffer.  "*" is not really part of the
buffer.  It denotes the position of point:


===File ~/bobbug.sh=========================================
case aa in
    aa)
    *
============================================================

Now do M-: (sh-get-indent-info)

"Beginning of buffer" error.  That is because sh-get-indent-info is
doing a (forward-char -1) at the beginning of the buffer.  I am not an
expert in shell indentation and while I am familiar with certain parts
of sh-script.el, I am not very familiar with the indentation
machinery.  However, the very simple change in the patch below seems
to fix the problem.

Many people seem to use the convention of starting shell scripts, even
ones intended to be sourced, with a comment, be it not a magic comment.
For instance, the first line of my .bashrc is:
# .bashrc.

In that case, the bug will never occur.  While in those cases my patch
is not necessary, it does no harm either.  If that convention is not
followed, the patch is necessary.

Change log:

2003-01-11  Luc Teirlinck  <teirllm@mail.auburn.edu>

        * sh-script.el (sh-get-indent-info):  Do not call
        (forward-char -1) at the beginning of the buffer. 

Patch:

===File ~/bobdiff===========================================
cd /usr/local/share/emacs/21.3.50/lisp/progmodes/
diff -c /usr/local/share/emacs/21.3.50/lisp/progmodes/sh-script.el /usr/local/share/emacs/21.3.50/lisp/progmodes/sh-script.new.el
*** /usr/local/share/emacs/21.3.50/lisp/progmodes/sh-script.el	Fri Jan 10 15:48:47 2003
--- /usr/local/share/emacs/21.3.50/lisp/progmodes/sh-script.new.el	Sat Jan 11 14:42:43 2003
***************
*** 2036,2042 ****
  		    (progn
  		      (setq result (append result val))
  		      (setq align-point (point))))
! 		(forward-char -1)
  		(skip-chars-forward "[a-z0-9]*?")
  		)
  	       ((string-match "[])}]" x)
--- 2036,2042 ----
  		    (progn
  		      (setq result (append result val))
  		      (setq align-point (point))))
! 		(unless (bobp) (forward-char -1))
  		(skip-chars-forward "[a-z0-9]*?")
  		)
  	       ((string-match "[])}]" x)

Diff finished at Sat Jan 11 14:43:03
============================================================

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Do not apply my previous patch for sh-get-indent-info.  It was too
simplistic.  I will give more details later.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Here is the problem with my previous patch:

The return value of sh-get-indent-info after my patch avoiding the
"Beginning of buffer error" (in the previously described situation)
is:

((t 1) (+ sh-indent-for-case-alt) (+ sh-indent-for-case-label))

Now we add:
#! /usr/local/bin/bash
and a blank line at the beginning of the buffer.

New return value (same relative position of point):

((t 25) (+ sh-indent-for-case-alt))

Adding only a blank line yields:

((t 2) (+ sh-indent-for-case-alt))

The 25 and 2 are expected.  The beginning of the case statement is now
at position 25 (2, respectively).  However, somehow the 
(+ sh-indent-for-case-label) in the first return value seems to be
a remaining bug.

Note that the entire bug never occurs if the file starts with any kind
of comment, magic or not, or with a blank line.  Even if we can take
care of this problem, I do not know whether there are no other places
in the indentation machinery that assume the file to start with some
kind of comment, or blank line.

Could somebody familiar with the shell indentation machinery take a
look at this?  The (forward-char -1) is where the problem manifests
itself.  Clearly at the beginning of the buffer we should definitely
not call (forward-char -1), but somehow for the remaining code to work
well point should be at "the end of the line before the beginning of
the buffer".

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Stefan Monnier wrote:

   I change skeleton.el to only add starting-\n and ending-\n when
   "needed" but it looks like it doesn't do what it should in your case.
   I'll take a look at it,

I have no idea what the original motivation for the change was, but
would it not be possible to correct the original problem in the
define-skeleton's rather than in skeleton.el, in which case the sh-if
and sh-case problem would automatically disappear, by reverting your
change?

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Whatever you decide to do with newlines in skeleton.el, I believe it
would be extremely useful if you could specify in full detail (for
instance in the documentation string of skeleton-insert) what the
exact effect of \n in a define-skeleton is.  The only desciption seems
to be in the documentation string of skeleton-insert:

\n     go to next line and indent according to mode

It does not say anything about starting-\n and ending-\n being treated
differently.  If you decide to revert things in such a way that the
above description is complete and accurate, there is no problem.
Otherwise, people should be able to find out exactly what the
define-skeleton machinery is supposed to do without having to read
through every single line of source code of skeleton.el.  The lack of
complete and accurate documentation is a big part of what makes
skeleton.el and its applications so difficult to understand.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Stefan Monnier wrote:

   I believe it is because of the following:
   in Emacs-20, sh-script did not offer auto-indentation so skeletons used
   a "rigid" form of indentation defined in the skeleton's definition.
   In Emacs-21, OTOH sh-script offers auto-indentation and skeletons thus
   simply rely on it to do their job.

   Now why was the indentation style chosen differently in the indentation
   code than what the skeletons used, I don't know.

What is meant by auto-indentation?

What definitely changed between 20.7 and 21 is that the
indentation-style is now very customizable for shells derived from sh
and rc.  That should not have affected the default indentation and
all my "Sh Indentation" variables are at their standard settings.

You seem to suggest that sh-mode can now "indent-as-you-go" while one
is typing in one's shell script, like one can do in the Lisp or C
modes, regardless of whether one uses sh-if, sh-case and friends.

Maybe I am missing something, but how does this work?

The only indentation related bound commands in sh-mode are TAB and
C-j.  The main one I use and I believe most people use to
"indent-as-you-go" in Lisp or C modes is C-j.  In sh-mode this is
bound to sh-newline-and-indent.  C-h f yields:

    sh-newline-and-indent is an interactive compiled Lisp function in
    `/usr/local/share/emacs/21.3.50/lisp/progmodes/sh-script.elc'.
    It is bound to C-j.
    (sh-newline-and-indent)

    Strip unquoted whitespace, insert newline, and indent like current
    line.

That seems to accurately describe the actual behavior.  The next line
just gets indented like the previous one.  Obviously, that is not
always correct.  It is also no different from Emacs20 behavior.

Did I misunderstand you?

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

>From my previous message:

   Maybe I am missing something, but how does this work?

I might have figured this one out myself.
TAB works, C-j does its own old thing.  (This seems inconsistent
with other programming modes, however.)  TAB does seem to put the 
;; in exactly the same place as Emacs20.7 used to do.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Stefan Monnier]

> Would it be possible for somebody more familiar with the skeleton.el
> machinery than I am to take a look at the following?  Something in the
> skeleton.el machinery seems to "eat newlines".
> 
> The actual manifestations of the problem (at least the ones known to
> me) occur in the sh-mode commands sh-for and sh-case, but I believe
> that some bug in skeleton.el is the actual underlying cause.

I believe the problem with missing newlines is now fixed.
There is still the problem of the ;; not being correctly indented.
I now think it's also a skeleton.el bug I introduced, but I haven't
tracked it down yet.


	Stefan

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Stefan Monnier wrote:

   I believe the problem with missing newlines is now fixed.
   There is still the problem of the ;; not being correctly indented.
   I now think it's also a skeleton.el bug I introduced, but I haven't
   tracked it down yet.

The problem seems indeed to be fixed up to the ;;-indentation.

While on the subject of skeleton.el, I believe there is an annoying
typo in the info.

Do:

m Autotype m Skeleton Language

Take a look at the following there:

`?\n'
     Insert a newline and align under current line.  Use newline
     character `?\n' to prevent alignment.

Clearly makes no sense: insert ?\n to align as well as to prevent
alignment.  Is that first `?' not a typo?  That is, is it true that \n
causes alignment (as the documentation string of skeleton-insert says)
and ?\n prevents alignment?  Depending on how you fixed skeleton.el,
some further explanation of the subtleties involved (if there still
would be subtleties involved) here and in the documentation string of
skeleton-insert might be necessary.

Sincerely,

Luc.

Re: skeleton.el machinery eats newlines.

[Stefan Monnier]

I think I've finally tracked down and fixed the ;;-indentation problem.
It turns out it was not in skeleton.el but in sh-script.el and was
a subtle issue of a `syntax-table' text-property being wrongly inherited
because of a missing `rear-nonsticky' property.


	Stefan

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

Stefan Monnier wrote:

   I think I've finally tracked down and fixed the ;;-indentation problem.
   It turns out it was not in skeleton.el but in sh-script.el and was
   a subtle issue of a `syntax-table' text-property being wrongly inherited
   because of a missing `rear-nonsticky' property.

sh-if and sh-case now indeed seem to work fine, indentation and all,
except when sh-case is invoked at the very beginning of a buffer, in
which case one runs into a "Beginning of buffer" error, but that is a
completely separate issue, on which I reported earlier in this thread,
related to the question of whether sh-script.el assumes that all
shell-scripts start with a magic number (or at least some kind of
comment) and whether it should actually do so.  I will follow up on
this separately since it is an unrelated issue.

Thanks,

Luc.

Re: skeleton.el machinery eats newlines.

[Luc Teirlinck]

I did not realize that the subject I initiated in another thread
(sh-script.el and magic numbers) had already been discussed before.
It was not my intention to rediscuss issues on which a decision had
already been made.  That decision was that sh-script.el should not
rely on shell scripts starting with any kind of comment, magic or
other.  It seems to me that the very fact that the subject seems
controversial indicates that this decision was correct, since we are
in the business of helping people write in whichever style they
choose, not in the business of prescribing programming styles (except,
of course, for stuff submitted for inclusion in Emacs).

Given that, however, we have at least one problem.  (I now believe
that the other two problems I referred to, but did not elaborate on,
can both be solved, by the user, by giving sh-shell-file a file local
value, so I will not go into these.)

This leaves the "Beginning of buffer" problem.  Trying sh-case
(normally bound to C-c C-c) at the very beginning of the buffer pretty
soon aborts with this error.  Nothing wrong with sh-case or skeletons,
`sh-get-indent-info' is to blame, as I reported earlier in this thread.
(I really should have made a separate thread out of this, it has
nothing to do with skeleton.el.)  I could repost my bug reports from
more than a week ago if this would seem useful.

The problem is, as I explained earlier, that sh-get-indent-info does a
(forward-char -1) at the beginning of the buffer.  I first (naively)
tried (unless (bobp) (forward-char -1)), but this was not good
enough.  For a correct return value (and hence for correct
indentation, in particular of the case form) point really needs to be
at position 0, just before the beginning of the buffer.

One obvious (but probably contorted and inelegant) way in which the
problem can be solved is to add a newline at the beginning of the
buffer at the start of sh-get-indent-info, erase it at the end of that
function and then replace the resulting (t NUMBER) by (t (1- NUMBER)).

A more elegant solution seems to require a good understanding of the
entire sh-script.el indentation mechanism.  At present, I do not have
the time to start studying that mechanism.  I have the impression that
anybody familiar with that mechanism could come up with a much more
elegant solution than the one described above.  However, I could
implement that solution if it would be considered acceptable and
nobody would come up with a better one.  (I believe that the only
problem with that solution is its stylistic inelegance.)

Sincerely,

Luc.