reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[nuxdoors]

Hi, In GNU Emacs 22.1:

reindent-then-newline-and-indent doesn't indent properly.

The command currently calls delete-horizontal-space after indenting the
line, which in effect deletes the re-indentation since at that very
moment the point is at the beginning of the text. If you swap the order
of the delete-horizontal-space and indent-according-to-mode functions it
then works as intended, like this :

(defun reindent-then-newline-and-indent ()
  "Reindent current line, insert newline, then indent the new line.
Indentation of both lines is done according to the current major mode,
which means calling the current value of `indent-line-function'.
In programming language modes, this is the same as TAB.
In some text modes, where TAB inserts a tab, this indents to the
column specified by the function `current-left-margin'."
  (interactive "*")
  (let ((pos (point)))
    ;; Be careful to insert the newline before indenting the line.
    ;; Otherwise, the indentation might be wrong.
    (newline)
    (save-excursion
      (goto-char pos)
      (delete-horizontal-space t)
      (indent-according-to-mode))
    (indent-according-to-mode)))

Regards,
Fabrice.

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[martin rudalics]

 > reindent-then-newline-and-indent doesn't indent properly.
 >
 > The command currently calls delete-horizontal-space after indenting the
 > line, which in effect deletes the re-indentation since at that very
 > moment the point is at the beginning of the text. If you swap the order
 > of the delete-horizontal-space and indent-according-to-mode functions it
 > then works as intended, like this :

`delete-horizontal-space' is needed to get rid of whitespace at the
_end_ of the old line, that is, after the position of `point' when you
invoked that command.  If `reindent-then-newline-and-indent' really
deletes whitespace inserted by `indent-according-to-mode' before text on
the old line there is a bug.  Can you give us a practical example where
this happens?

 > (defun reindent-then-newline-and-indent ()
 >   "Reindent current line, insert newline, then indent the new line.
 > Indentation of both lines is done according to the current major mode,
 > which means calling the current value of `indent-line-function'.
 > In programming language modes, this is the same as TAB.
 > In some text modes, where TAB inserts a tab, this indents to the
 > column specified by the function `current-left-margin'."
 >   (interactive "*")
 >   (let ((pos (point)))
 >     ;; Be careful to insert the newline before indenting the line.
 >     ;; Otherwise, the indentation might be wrong.
 >     (newline)
 >     (save-excursion
 >       (goto-char pos)
 >       (delete-horizontal-space t)
 >       (indent-according-to-mode))
 >     (indent-according-to-mode)))

The problem with this is that the first `indent-according-to-mode' may
insert whitespace at the end of an otherwise empty line.  Try to invoke
your function at the beginning of a non-empty line.

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[nuxdoors]

Ok the problem is not occuring in the official version of emacs 22.1, at
least not in the same obvious way. Sorry about the alarmous report. I
still have found a minor bug related to this in the official version
though, described below.

<my life>
I have found the problem to be much more obvious in the latest testing
emacs version (22.1+1-2) and will thus file a bug report at debian.org

I reported here because i saw the reindent-then-newline-and-indent
function on the Savannah CVS Surfing service (which was the same as the
one found in my emacs) was making the assumption that
indent-according-to-mode was indenting without moving the cursor
position, which i thought could be the source of the problem.

It turns out this is the source of the problem for my debian version,
where i use a develock advised version of lisp-indent-line which leaves
the cursor at the beginning of text, just after the indentation
characters. It doesn't happen in the official version though because it
seems pretty every indentation function included in the various modes
keeps the cursor in place ( i didn't test all the modes though of course ).
</my life>

In respect to the official emacs version, here is the description of a
bug with indent-to-left-margin (which used to be the default indentation
function for the Fundamental Mode so although this isn't even a command
and thus not available to alt-x some people might still use it ) :

Let's create indentation_test.txt like this :

cat > indentation_test.txt <<EOF
    First line. Second line to be indented like the first one.

Local Variables:
left-margin:4
indent-line-function:indent-to-left-margin
End:
EOF

Then i called emacs compiled from source like this :
emacs -Q -nw indentation_test.txt

answer yes at the local variable question.

The "|" character represents the cursor position.
Move you cursor to/on the "S" letter like this :

    First line. |Second line to be indented like the first one.

Then do a reindent-then-newline-and-indent, you will get this :

First line.
    |Second line to be indented like the first one.

This isn't visible but there is still a space at the end of the first
line. I believe we should get this :

    First line.
    |Second line to be indented like the first one.

(no space at the end of the first line and the "S" is on the same column
as "F")

The problem arises because indent-to-left-margin is one of the rare
indentation functions who leaves the cursor at the beginning of the text
after indenting.

So if we think that an indentation function should always leave the
cursor in place, i guess this isn't a bug and we can say
indent-to-left-margin is at fault here ( or the user who doesn't use a
more recent indentation function ).
On the other hand i have been reading the info docs and string docs in
indent.el for example and i can't find a place where it is said that an
indentation function should always preserve the cursor position. The
proper functionning of reindent-then-newline-and-indent is based on that
assumption, or at least that indent-according-to-mode respect that, but
it is not stated in the indent-according-to-mode's doc string that it
should.

I think either that statement should be made somewhere or
reindent-then-newline-and-indent shouldn't make that assumption, it
could for example encapsulate the indent-according-to-mode call itself
in a save-excursion form, which would solve the issue.

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[nuxdoors]

Here is the patch alone, hopefully without thunderbird mangling it.

--- lisp/simple.el.orig 2007-05-27 16:35:51.000000000 +0200
+++ lisp/simple.el      2007-10-14 15:21:13.000000000 +0200
@@ -618,7 +618,8 @@
   (indent-according-to-mode))

 (defun reindent-then-newline-and-indent ()
-  "Reindent current line, insert newline, then indent the new line.
+  "Reindent current line, insert newline deleting trailing spaces or tabs,
+then indent the new line.
 Indentation of both lines is done according to the current major mode,
 which means calling the current value of `indent-line-function'.
 In programming language modes, this is the same as TAB.
@@ -631,8 +632,12 @@
     (newline)
     (save-excursion
       (goto-char pos)
-      (indent-according-to-mode)
-      (delete-horizontal-space t))
+      ;; Beware of indentation functions moving point.
+      (save-excursion
+       (indent-according-to-mode))
+      ;; Delete all trailing spaces or tabs introduced by the newline command
+      ;; or the new indentation. 0 is for no space.
+      (just-one-space 0))
     (indent-according-to-mode)))

 (defun quoted-insert (arg)

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[martin rudalics]

 > Here is the patch alone, hopefully without thunderbird mangling it.

With Thunderbird better send patches as attachments.

 >
 > --- lisp/simple.el.orig 2007-05-27 16:35:51.000000000 +0200
 > +++ lisp/simple.el      2007-10-14 15:21:13.000000000 +0200
 > @@ -618,7 +618,8 @@
 >    (indent-according-to-mode))
 >
 >  (defun reindent-then-newline-and-indent ()
 > -  "Reindent current line, insert newline, then indent the new line.
 > +  "Reindent current line, insert newline deleting trailing spaces or tabs,
 > +then indent the new line.

Note that the first sentence of a doc-string should fit on one line.

 >  Indentation of both lines is done according to the current major mode,
 >  which means calling the current value of `indent-line-function'.
 >  In programming language modes, this is the same as TAB.
 > @@ -631,8 +632,12 @@
 >      (newline)
 >      (save-excursion
 >        (goto-char pos)
 > -      (indent-according-to-mode)
 > -      (delete-horizontal-space t))
 > +      ;; Beware of indentation functions moving point.
 > +      (save-excursion
 > +       (indent-according-to-mode))
 > +      ;; Delete all trailing spaces or tabs introduced by the newline command
 > +      ;; or the new indentation. 0 is for no space.
 > +      (just-one-space 0))

Why is this better than `delete-horizontal-space'?

 >      (indent-according-to-mode)))
 >
 >  (defun quoted-insert (arg)

In any case you might have to convince other people first that your
solution is preferable to using, for example,

   (save-excursion (indent-line-to (current-left-margin))))

as `indent-line-function'.

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[Stefan Monnier]

> --- lisp/simple.el.orig 2007-05-27 16:35:51.000000000 +0200
> +++ lisp/simple.el      2007-10-14 15:21:13.000000000 +0200
> @@ -618,7 +618,8 @@
>    (indent-according-to-mode))

>  (defun reindent-then-newline-and-indent ()
> -  "Reindent current line, insert newline, then indent the new line.
> +  "Reindent current line, insert newline deleting trailing spaces or tabs,
> +then indent the new line.
>  Indentation of both lines is done according to the current major mode,
>  which means calling the current value of `indent-line-function'.
>  In programming language modes, this is the same as TAB.
> @@ -631,8 +632,12 @@
>      (newline)
>      (save-excursion
>        (goto-char pos)
> -      (indent-according-to-mode)
> -      (delete-horizontal-space t))
> +      ;; Beware of indentation functions moving point.
> +      (save-excursion
> +       (indent-according-to-mode))
> +      ;; Delete all trailing spaces or tabs introduced by the newline command
> +      ;; or the new indentation. 0 is for no space.
> +      (just-one-space 0))
>      (indent-according-to-mode)))

>  (defun quoted-insert (arg)


The patch looks OK w.r.t wrapping indent-according-to-mode in save-excursion
(indeed I do expect all indentation functions to "preserve" point but it's
not stated anywhere, so I think both reindent-then-newline-and-indent and
indent-to-left-margin are at fault here).

But I do not understand why you replaced delete-horizontal-space with
just-one-space.


        Stefan

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[Stefan Monnier]

> Here is the patch alone, hopefully without thunderbird mangling it.

I've installed a similar patch, thank you,


        Stefan

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[Richard Stallman]

Does this version fix the bug?

(defun indent-to-left-margin ()
  "Indent current line to the column given by `current-left-margin'."
  (save-excursion (indent-line-to (current-left-margin)))
  ;; If we are within the indentation, move past it.
  (when (save-excursion
	  (skip-chars-backward " \t")
	  (bolp))
    (beginning-of-line)
    (skip-chars-forward " \t")))

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[nuxdoors]

martin rudalics wrote:
> Note that the first sentence of a doc-string should fit on one line.
Note taken, thanks.

Is this sentence understandable/acceptable to add at the end of the doc
string ?
"Ensures no space or tab remains at the end of first line."

Stefan Monnier wrote:
> But I do not understand why you replaced delete-horizontal-space with
> just-one-space.

Well... With some late at night symetric reasoning ( which makes me
think it is late again ) i figured if delete-horizontal-space deletes
backward with a t argument then maybe it will delete forward with a nil
argument ( don't ask ). I was just too lazy to ckeck it up and used
just-one-space which i knew would do the job for sure.
delete-horizontal-space is all right of course.

Fabrice.

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[Richard Stallman]

    Well... With some late at night symetric reasoning ( which makes me
    think it is late again ) i figured if delete-horizontal-space deletes
    backward with a t argument then maybe it will delete forward with a nil
    argument ( don't ask ).

There is no reason to delete spaces after point.
Those will end up at the start of the next line
and indentation will adjust them.

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[nuxdoors]

Stefan Monnier wrote:
> I've installed a similar patch, thank you,
Thank you, this is very much appreciated.

You need to delete both spaces ways though. That's why i chose to use
(just-one-space 0) over (delete-horizontal-space t) when i didn't
realise that (delete-horizontal-space) would do the same thing.

Richard Stallman wrote:
> There is no reason to delete spaces after point.
> Those will end up at the start of the next line
> and indentation will adjust them.

If you want to keep the same behaviour as before you need to delete both
ways. That is needed for the special case where the function is called
while the point is whithin the line indentation. In this case the newly
introduced save-excursion form might leave the point in the middle of
white spaces. If you then delete only backward, spaces will be left on
the line.

Fabrice

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[nuxdoors]

Here is a concrete example to illustrate my previous remark with respect
to the need of removing spaces around point when calling
delete-horizontal-space.

Tested with GNU Emacs 23.0.50.1 (with the new
reindent-then-newline-and-indent)

Let's create indentation_test.txt like this :

cat > indentation_test.txt <<EOF
    First line.
    Second line.
1234 -- columns
EOF

emacs was called like this :
emacs -Q -nw indentation_test.txt

"_" represents a space, "|" represents cursor position.
Place the cursor on the fourth column ( on the space preceding "S" ),
like this :

____First line.
___|Second line.
1234 -- columns

Then do a reindent-then-newline-and-indent, you will get this :

____First line.
_
____|Second line.
1234 -- columns

With the previous version of reindent-then-newline-and-indent you would
get this :

____First line.

____|Second line.
1234 -- columns

The patch to restore previous behavior that works for me :

*** lisp/simple.el.orig Thu Oct 18 06:52:06 2007
--- lisp/simple.el      Fri Oct 19 02:41:22 2007
***************
*** 636,642 ****
        ;; Usually indent-according-to-mode should "preserve" point, but
it is
        ;; not guaranteed; e.g. indent-to-left-margin doesn't.
        (save-excursion (indent-according-to-mode))
!       (delete-horizontal-space t))
      (indent-according-to-mode)))

  (defun quoted-insert (arg)
--- 636,644 ----
        ;; Usually indent-according-to-mode should "preserve" point, but
it is
        ;; not guaranteed; e.g. indent-to-left-margin doesn't.
        (save-excursion (indent-according-to-mode))
!       ;; Delete forward too in case reindent-then-newline-and-indent
!       ;; was called while point was within the line indentation.
!       (delete-horizontal-space))
      (indent-according-to-mode)))

  (defun quoted-insert (arg)

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[nuxdoors]

> The problem with this is that the first `indent-according-to-mode' may
> insert whitespace at the end of an otherwise empty line.  Try to invoke
> your function at the beginning of a non-empty line.

I take care of this in this new function by deleting spaces after
indenting the line around the point of initial breakage. I expect it has
now the same behaviour as before except for the minor bug. Please test.

I completed the doc string with a note one the automatic trailing space
deleting. Shouldn't that be stated or is it too obvious ?

(defun reindent-then-newline-and-indent ()
  "Reindent current line, insert newline deleting trailing spaces or tabs,
then indent the new line.
Indentation of both lines is done according to the current major mode,
which means calling the current value of `indent-line-function'.
In programming language modes, this is the same as TAB.
In some text modes, where TAB inserts a tab, this indents to the
column specified by the function `current-left-margin'."
  (interactive "*")
  (let ((pos (point)))
    ;; Be careful to insert the newline before indenting the line.
    ;; Otherwise, the indentation might be wrong.
    (newline)
    (save-excursion
      (goto-char pos)
      ;; Beware of indentation functions which move point.
      (save-excursion
    (indent-according-to-mode))
      ;; Delete all trailing spaces or tabs introduced by the newline
command
      ;; or the new indentation. 0 is for no space.
      (just-one-space 0))
    (indent-according-to-mode)))

-----------------------------------------

--- lisp/simple.el.orig 2007-05-27 16:35:51.000000000 +0200
+++ lisp/simple.el      2007-10-14 14:38:20.000000000 +0200
@@ -618,7 +618,8 @@
   (indent-according-to-mode))

 (defun reindent-then-newline-and-indent ()
-  "Reindent current line, insert newline, then indent the new line.
+  "Reindent current line, insert newline deleting trailing spaces or tabs,
+then indent the new line.
 Indentation of both lines is done according to the current major mode,
 which means calling the current value of `indent-line-function'.
 In programming language modes, this is the same as TAB.
@@ -631,8 +632,12 @@
     (newline)
     (save-excursion
       (goto-char pos)
-      (indent-according-to-mode)
-      (delete-horizontal-space t))
+      ;; Beware of indentation functions which move point.
+      (save-excursion
+       (indent-according-to-mode))
+      ;; Delete all trailing spaces or tabs introduced by the newline
command
+      ;; or the new indentation.
+      (just-one-space 0))
     (indent-according-to-mode)))

 (defun quoted-insert (arg)

Re: reindent-then-newline-and-indent doesn't indent properly in emacs 22.1

[Richard Stallman]

I'm inclined to change `indent-to-left-margin' because that results in
more uniformity for all kinds of indentation.

At the same time, I don't see why your original change would be bad.
More precisely, I don't see a case in which the change would make the
behavior worse.  Can anyone else point out such a case?