From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.devel Subject: electric-indent-post-self-insert-function: a partial code review. Date: Sun, 17 Nov 2013 16:27:38 +0000 Message-ID: <20131117162738.GA19455@acm.acm> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1384705819 18801 80.91.229.3 (17 Nov 2013 16:30:19 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 17 Nov 2013 16:30:19 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Nov 17 17:30:23 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 1Vi5Ep-0004Du-72 for ged-emacs-devel@m.gmane.org; Sun, 17 Nov 2013 17:30:19 +0100 Original-Received: from localhost ([::1]:39547 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vi5Eo-0000ny-Mg for ged-emacs-devel@m.gmane.org; Sun, 17 Nov 2013 11:30:18 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:41830) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vi5Ee-0000l8-8w for emacs-devel@gnu.org; Sun, 17 Nov 2013 11:30:15 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vi5EW-00010u-Pk for emacs-devel@gnu.org; Sun, 17 Nov 2013 11:30:08 -0500 Original-Received: from colin.muc.de ([193.149.48.1]:20444 helo=mail.muc.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vi5EW-00010e-FX for emacs-devel@gnu.org; Sun, 17 Nov 2013 11:30:00 -0500 Original-Received: (qmail 39373 invoked by uid 3782); 17 Nov 2013 16:29:58 -0000 Original-Received: from acm.muc.de (pD9518CC8.dip0.t-ipconnect.de [217.81.140.200]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 17 Nov 2013 17:29:57 +0100 Original-Received: (qmail 20131 invoked by uid 1000); 17 Nov 2013 16:27:38 -0000 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-detected-operating-system: by eggs.gnu.org: FreeBSD 8.x X-Received-From: 193.149.48.1 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:165299 Archived-At: 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 ). 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).