emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Monnier
Subject: Re: electric-indent-post-self-insert-function: a partial code review.
Date: Sun, 17 Nov 2013 16:11:07 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux)

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

Feel free to improve it, yes.

> #########################################################################
> 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"?

"Reindentation" is to adjust the indentation of an existing line.
It is premised on the idea that indent-line-function will "always" give
a result that's no worse than the current line's indentation.

In contrast, "indentation" in this context is when space is added on
a line that is not pre-existing, so in a sense it can't be worse than
what was there before, since there was nothing before.

Think of "reindent-then-newline-and-indent".

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

There are also two calls in "reindent-then-newline-and-indent", the
first is the "reindent", then other is the "indent".

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

When the code is run last-command-event should be 10 in both cases:
`newline' let-binds last-command-event to 10.

>     What is this form intended to distinguish?

electric--after-char-pos' is all about finding the self-inserted char,
which is not always right before point, since abbrev-expansion and
post-self-insert-hook functions may have made arbitrary changes since
the char was inserted.

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

If you don't want that, then I suggest you disable electric-indent-mode.

> regardless of `electric-indent-inhibit'.  This is surely a bug.

No, it's by design.  Otherwise electric-indent-inhibit would just disable
electric-indent-mode, which would be redundant: you can already do that
by ... turning it off.

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

I wrote it as well as I could.  If you can make it simpler without
breaking the behavior, please do so.

Some of the interesting cases come up when it gets combined with things
like electric-pair-mode or electric-layout-mode.


        Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]