electric-indent-post-self-insert-function: a partial code review.

[Alan Mackenzie <acm@muc.de>]

Hi, Emacs.

The doc string of `electric-indent-inhibit' says:

    "If non-nil, reindentation is not appropriate for this buffer."

.  This is vague and wishy-washy.  What, exactly, does "not appropriate"
mean?  When non-nil, does reindentation get done, or doesn't it?  Better
would be:

    "If non-nil, electric reindentation is not done in this buffer."

, if this is in fact what is intended.

#########################################################################

The doc string of `electric-indent-functions-without-reindent' says:

    "List of indent functions that can't reindent."

.  Even though the rest of the doc string explains what is meant, this
top line is nonsensical - all the functions listed _can_ reindent.
Better, I think, would be:

    "List of indent functions which won't be used for reindentation."

, even if not all that much better.  But what is meant by
"REindentation", as opposed to "indentation"?

Also, in `electric-indent-post-self-insert-function', there are two
calls to `indent-according-to-mode'.  `e-i-f-without-reindent' is only
checked for one of these calls.  Is this a bug?

########################################################################

In `electric--after-char-pos', there is the strange looking form:

    (eq (char-before) last-command-event) ;; Sanity check.

.  What is this supposed to check?  After inserting a newline,
(char-before) is 10.  `last-command-event' is (on my Linux tty) either
10 or 13 (after typing C-j or <ret>).  Distingushing them here doesn't
seem to make sense.  What is this form intended to distinguish?

#########################################################################

Assuming the above meaning for `electric-indent-inhibit', then there is
the following problem: even with `e-i-inhibit' set to t, electric
indentation gets done on the new line after insertion of a \n.

To see this, note that `pos' is bound to (electric--after-char-pos),
that is, the position after the last non-ws character inserted into the
buffer.  The last clause inside the outermost `when' is:

      (unless (and electric-indent-inhibit
                   (> pos (line-beginning-position)))
        (indent-according-to-mode)))

.  This will invoke `indent-according-to-mode' when (<= pos
(line-beginning-position)), i.e. when a \n has just been inserted,
regardless of `electric-indent-inhibit'.  This is surely a bug.

#########################################################################

In general, `electric-mode-post-self-insert-function' seems horrifically
and needlessly over-complicated, even though it is only 50 lines long.
The various checks performed before invoking `indent-according-to-mode'
are done at many different places at several different levels of nesting
in the code.  For instance, why is `electric-indent-inhibit' checked
twice at a lower level, rather than just once at the top level?

Why can the various checks not simply be successive arms of an `and'
form?

The reindentation of the original line sometimes happens in the first
call of `indent-according-to-mode', sometimes in the second call.
Perhaps it would be clearer if the original line was always reindented
in the first call, and the new line (if any) in the second call.

-- 
Alan Mackenzie (Nuremberg, Germany).