From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: electric-indent-post-self-insert-function: a partial code review. Date: Sun, 17 Nov 2013 16:11:07 -0500 Message-ID: References: <20131117162738.GA19455@acm.acm> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1384722686 26517 80.91.229.3 (17 Nov 2013 21:11:26 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 17 Nov 2013 21:11:26 +0000 (UTC) Cc: emacs-devel@gnu.org To: Alan Mackenzie Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Nov 17 22:11:30 2013 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Vi9cv-0005i4-3B for ged-emacs-devel@m.gmane.org; Sun, 17 Nov 2013 22:11:29 +0100 Original-Received: from localhost ([::1]:40320 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vi9cu-0003EW-Ie for ged-emacs-devel@m.gmane.org; Sun, 17 Nov 2013 16:11:28 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:55300) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vi9cj-0003CN-Mv for emacs-devel@gnu.org; Sun, 17 Nov 2013 16:11:25 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vi9ca-00030G-4z for emacs-devel@gnu.org; Sun, 17 Nov 2013 16:11:17 -0500 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.182]:3828) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vi9ca-00030C-0h for emacs-devel@gnu.org; Sun, 17 Nov 2013 16:11:08 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Av4EABK/CFFFpZ/0/2dsb2JhbABEvw4Xc4IeAQEEAVYjBQsLDiYSFBgNJIgeBsEtkQoDiGGcGYFegxU X-IPAS-Result: Av4EABK/CFFFpZ/0/2dsb2JhbABEvw4Xc4IeAQEEAVYjBQsLDiYSFBgNJIgeBsEtkQoDiGGcGYFegxU X-IronPort-AV: E=Sophos;i="4.84,565,1355115600"; d="scan'208";a="38179611" Original-Received: from 69-165-159-244.dsl.teksavvy.com (HELO pastel.home) ([69.165.159.244]) by ironport2-out.teksavvy.com with ESMTP/TLS/ADH-AES256-SHA; 17 Nov 2013 16:11:07 -0500 Original-Received: by pastel.home (Postfix, from userid 20848) id 3E15361245; Sun, 17 Nov 2013 16:11:07 -0500 (EST) In-Reply-To: <20131117162738.GA19455@acm.acm> (Alan Mackenzie's message of "Sun, 17 Nov 2013 16:27:38 +0000") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.182 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:165307 Archived-At: > "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 ). 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