From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: joaotavora@gmail.com (=?utf-8?B?Sm/Do28gVMOhdm9yYQ==?=) Newsgroups: gmane.emacs.devel Subject: Re: [patch] make electric-pair-mode smarter/more useful Date: Fri, 13 Dec 2013 01:02:41 +0000 Message-ID: <87r49h7eby.fsf@gmail.com> References: <87haalh806.fsf@gmail.com> <87ppp2ydqf.fsf@gmail.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1386896654 28707 80.91.229.3 (13 Dec 2013 01:04:14 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 13 Dec 2013 01:04:14 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Dec 13 02:04:19 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 1VrHAx-0005Ja-41 for ged-emacs-devel@m.gmane.org; Fri, 13 Dec 2013 02:04:19 +0100 Original-Received: from localhost ([::1]:39344 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrHAw-0000Hr-La for ged-emacs-devel@m.gmane.org; Thu, 12 Dec 2013 20:04:18 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:54487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrHAp-0000HZ-CV for emacs-devel@gnu.org; Thu, 12 Dec 2013 20:04:16 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VrHAk-000641-Au for emacs-devel@gnu.org; Thu, 12 Dec 2013 20:04:11 -0500 Original-Received: from mail-wg0-x234.google.com ([2a00:1450:400c:c00::234]:52171) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrHAk-00063q-0f for emacs-devel@gnu.org; Thu, 12 Dec 2013 20:04:06 -0500 Original-Received: by mail-wg0-f52.google.com with SMTP id x13so1219845wgg.31 for ; Thu, 12 Dec 2013 17:04:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:in-reply-to:references:user-agent:date :message-id:mime-version:content-type:content-transfer-encoding; bh=/6vTAKROuHq+xXwavRUeW3iSeCTxOAa7BgjVKMr27JU=; b=GfbJY/VbH7bXlC/RDAWu1LI7YVI2pm0k4mKJ8Mz1O/G4Tiq08XLkvCOy4N7Pp2Fohr vOXGB7D7Kuuu8YWGWmECbyaxBT4yWLBGdnntHGx8OYo1vGFgLqPSId/yHnVtgLcDBrI4 9Pw6tbuAN0XZ3603PP6kuAYBLurJA0inYqcpHMZzJrOrevg3k3GG9AayfPn3H64zs7+/ h915lVvVl7g8UAWrTBr+Lbjh3tc2pBWzIqIguBtGBYZdbK5P2p4dHW0QoMu1drFvxaA1 twv9wUj1o3HEtRKX0wEqNBJ6ArHgt5VqQzFCxm/u3297fJs/eejql6jditOMc+lNgqcK zlEQ== X-Received: by 10.194.201.225 with SMTP id kd1mr9102092wjc.35.1386896645109; Thu, 12 Dec 2013 17:04:05 -0800 (PST) Original-Received: from kitaj.yourcompany.com (66.207.108.93.rev.vodafone.pt. [93.108.207.66]) by mx.google.com with ESMTPSA id bc5sm894845wib.4.2013.12.12.17.04.03 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 12 Dec 2013 17:04:04 -0800 (PST) In-Reply-To: (Stefan Monnier's message of "Thu, 12 Dec 2013 13:08:55 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c00::234 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:166357 Archived-At: Stefan Monnier writes: > I think this change is for the worst. I often rely on ".." pairing in > things like text-mode where they don't have `string' syntax. But if you "rely" on those in text-mode, isn't the correct thing to give them that syntax there? > The explanation for removing (?\" . ?\") doesn't make much sense: you > seem to say that the list should be kept empty because balance can't be > provided, but until now balance wasn't provided either. I suggested in the docstring that before adding to this list, since it has priority, the lower priority buffer's syntax table should be used instead, as it also helps balancing. > Of course, when the syntax-table gives string syntax to ", then this > entry should be ignored since the syntax table gives more info, which > allows balancing. So you mean inverting the lookup order? Lookup electric-pair-pairs and electric-pair-text-pairs only if no relevant syntax is found in (syntax-table) and `electric-pair-text-syntax-table' respectively. If so, it seems like a good idea. I got the opposite idea from the original implementation of `electric-pair-syntax' and the mention of "regardless of major mode" in `electric-pair-pairs''s docstring. > I suggest you use "text" rather than "non-code" in the variable names > (and indeed, the behavior in strings and comments should largely be the > same as in text-mode). I disagree, but won't insist. "strings" and "comments" exist when programming, in my experience these typically are places where programmatic syntax is useful, to write error messages, explain structure, etc. But since `electric-pair-text-syntax-table' stays I'll just set it globablly to prog-mode-syntax-table in my config. This way I can get half-decent balancing for it. >> you provide examples of the conflicts between `electric-indent-mode' >> and other `electric-modes' that you mention there? > I can't remember off-hand, sorry. I think I just found one when implementing the `electric-layout-rules' suggestion in the nearby thread. > We can use syntax-ppss to get the START of all the parens, but indeed, > to find the overall balance, we need to look at least for the close-paren > matching the outermost open paren, which syntax-ppss doesn't give us. OK, so I'm already using `syntax-ppss' where possible right? >> In this implementation, I think the the number of `forward-sexp''s is >> at most proportional to the nesting depth, which is less >> dangerous. > Why do you need more than a fixed number of calls? My naive > understanding is that we only need to know if we have more openers than > closers, or more closers than openers, or the same number of each; and > for that we basically need to do: > > (goto-char (car (last (nth 9 (syntax-ppss))))) > (forward-sexp 1) =3D> failure means more openers than closers > (up-list 1) =3D> failure means the parens are balanced. I tried to make this into a function and failed again. > (save-excursion (car (syntax-ppss (point-max)))) This works... but only for one kind of parenthesis. It incorrectly reports 0 for a buffer with '[)', the mixed parenthesis case. I think none of these alternatives will make the "mixed parens" test cases pass, i.e. the ones where one kind is unbalanced but you still want to keep balancing the other kind. The motivation to invest in electric.el came after I fixed one outstanding such bug in autopair.el. > So I think we need a "electric-pair-preserve-balance" flag to control > those features. OK, but I would set it to t. Or maybe run `check-parens' in prog-mode-hook and set it to nil if that errors out. Maybe issue a warning to the user. By the way, even with that evil #ifdef that breaks balancing of the `{}' parens, we should strive to to still have `()' pair as if nothing happened. (currently this is still shaky). >> - some helper functions might be reinventing the wheel, such as >> `electric-pair--looking-at-mismatched-string-p' and > > IIUC if the next string is "mismatched" that means it extends til EOB, > so you can test it with (nth 3 (syntax-ppss (point-max))). Looks much simpler, but weirdly fails some automated tests while passing the same tests when run interactively. Will investigate. > IOW the problem is not in the implementation but in the idea of > detecting a string inside a comment. Yes. This was always slightly shaky in autopair.el, but good enough for most uses. It's seldom a problem. Anyway I'll try your suggestion. >> +(defun electric--sort-post-self-insertion-hook () > I think the idea is OK, but be careful: these functions are > supposed to be placed on the global part of the hook, so you'll want to > use `default-value' and `setq-default'. OK, good idea. > The priorities can be added as symbol properties, so the "sort" function > doesn't need to know about the existing hooks. Sounds a bit overkill for this particular case, unless we make it more generic, like maybe adding this to add-hook's understanding of its APPEND arg. But OK. >> +(make-variable-buffer-local 'electric-pair-non-code-pairs) > I think we don't want/need that. We can and do use setq-local when > needed, which should be sufficient. OK. >> +(defcustom electric-pair-skip-whitespace t >> + "If non-nil skip whitespace when skipping over closing parens. > > Unclear. IIUC from the code, what this doesn't just skip whitespace but > deletes it. It should also say which whitespace is skipped/deleted > (before/after the skipped paren). OK. Only the defcustom docs explain the whole story: it only deletes it when set to 'chomp, the default is t. >> +(defvar electric-pair-non-code-syntax-table prog-mode-syntax-table > Why prog-mode-syntax-table, rather than (say) text-mode-syntax-table? Explained above, but I don't object to text-mode-syntax-table. > BTW, syntax-ppss will get majorly confused if you call it while > a different syntax-table is temporarily installed. Never bit me, but thanks for the heads-up. I'll probably end up calling parse-partial-sexp with the correct table in comments or strings. > Don't use this :keymap arg; instead defvar electric-pair-mode-map. OK. >> + (setq-local electric-pair-skip-whitespace 'chomp)) > > Hmm... lemme think... well... maybe we can't keep it tentatively. > I suspect it will bite me sometimes, but let's give it a chance. I don't undestand: we "can" or we "can't" keep it? I think we should give it a chance for lisp based modes. elecric-mode is always going to be a surprise, might as well make it a good one. And when could it possibly bite you? >> - (newline) >> + (newline 1 (not (or executing-kbd-macro noninteractive))) >> (indent-according-to-mode)) > > Could you explain this change? No, it doesn't belong here, sorry :-) Some earlier attempt at making the related newline-between-pairs feature. But since you spotted it, shoudn't newline-and-indent also call the post-self-insertion hooks? Or should we leave that job to electric-indent-mode? Jo=C3=A3o