From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode Date: Fri, 09 Nov 2012 09:12:04 -0500 Message-ID: References: <87391ieck9.fsf@gmail.com> <87mwzdrly5.fsf@gmail.com> <87sj8s8bt1.fsf@gmail.com> <87obj7bdy6.fsf@gmail.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1352470386 22856 80.91.229.3 (9 Nov 2012 14:13:06 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 9 Nov 2012 14:13:06 +0000 (UTC) Cc: 12638@debbugs.gnu.org To: Jambunathan K Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Nov 09 15:13:16 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1TWpKd-0002Qx-Lg for geb-bug-gnu-emacs@m.gmane.org; Fri, 09 Nov 2012 15:13:15 +0100 Original-Received: from localhost ([::1]:48322 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TWpKU-0002zr-2W for geb-bug-gnu-emacs@m.gmane.org; Fri, 09 Nov 2012 09:13:06 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:36593) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TWpKM-0002mC-Qq for bug-gnu-emacs@gnu.org; Fri, 09 Nov 2012 09:13:03 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TWpKL-00005B-DC for bug-gnu-emacs@gnu.org; Fri, 09 Nov 2012 09:12:58 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:47080) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TWpKL-000057-9q for bug-gnu-emacs@gnu.org; Fri, 09 Nov 2012 09:12:57 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1TWpKQ-0001Lw-F5 for bug-gnu-emacs@gnu.org; Fri, 09 Nov 2012 09:13:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 09 Nov 2012 14:13:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 12638 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 12638-submit@debbugs.gnu.org id=B12638.13524703565167 (code B ref 12638); Fri, 09 Nov 2012 14:13:02 +0000 Original-Received: (at 12638) by debbugs.gnu.org; 9 Nov 2012 14:12:36 +0000 Original-Received: from localhost ([127.0.0.1]:57331 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TWpJl-0001L3-Kq for submit@debbugs.gnu.org; Fri, 09 Nov 2012 09:12:36 -0500 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.182]:45625) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TWpJd-0001Km-Kh for 12638@debbugs.gnu.org; Fri, 09 Nov 2012 09:12:15 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Ai0FAG6Zu09sr+ZY/2dsb2JhbABEsEiDSYEIghUBAQQBViMFCws0EhQYDSSIHAW6CZBEA4hCmnGBWIMH X-IronPort-AV: E=Sophos;i="4.75,637,1330923600"; d="scan'208";a="206806668" Original-Received: from 108-175-230-88.dsl.teksavvy.com (HELO pastel.home) ([108.175.230.88]) by ironport2-out.teksavvy.com with ESMTP/TLS/ADH-AES256-SHA; 09 Nov 2012 09:12:05 -0500 Original-Received: by pastel.home (Postfix, from userid 20848) id A0A905978D; Fri, 9 Nov 2012 09:12:04 -0500 (EST) In-Reply-To: <87obj7bdy6.fsf@gmail.com> (Jambunathan K.'s message of "Fri, 09 Nov 2012 09:55:05 +0530") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:66685 Archived-At: > 1. `minibuffer-summarize-completions' is a re-write of > `icomplete-exhibit' and line-by-line diff is not possible. Are there behavior differences? If so, can you summarize them (pun intended) before I try to understand the code? > 2. `minibuffer-summarize-completions' belongs in minibuffer.el and not > icomplete. Why? > It wouldn't because it is re-written. I am not sure whether you like > the re-write but it was definitely begging for re-write. (Please > confirm your preference here, so that I know what is acceptable.) I'm not necessarily opposed to a rewrite, but I think such a rewrite should aim to split it into smaller functions: the main problem with icomplete-exhibit is that it's too large. > The new re-write would make it easy to extract the summary builder to > it's own "message function" potentially along the lines of > `:exit-function' and `:annotation-function'. Is that intended to explain why you want to move it to minibuffer.el? If so, I don't understand it. Let's try to focus on improving icomplete for now. If/when some code in minibuffer.el could make use of some code in icomplete.el it'll be easy enough to move the code. If you move icomplete-exhibit (or its replacement) to minibuffer.el then you may as well move the whole of icomplete.el to minibuffer.el, and I'm not interested in doing that, for now. >> Agreed. I don't like it very much, especially because it is very ad-hoc >> (and worse, the code can be triggered in cases where it makes no sense >> to provide those shortcuts). > I will remove it in the next round and obsolete the corresponding > variable. If the variable doesn't work at all, then just remove it. "Obsolete" is used for things that are planned for removal but that still work. >>> - (set (make-local-variable 'completion-show-inline-help) nil) >>> + ;; (set (make-local-variable 'completion-show-inline-help) nil) >> Why? > Just wanted to bring attention to the fact that icomplete uses it's own > overlay. It is through this variable it shuts down minibuffer's very > own overlay. I remember this part. But if we keep such a reorganization for later, we still need this `set' here, right? >>> + (unless (memq this-command '(minibuffer-force-complete-and-exit minibuffer-forward-sorted-completions >>> + minibuffer-backward-sorted-completions)) >>> + ;; Current command does not belong to icomplete-mode. >>> + ;; Delete the overlay. >>> + (delete-overlay icomplete-overlay))) >> Why do you need such ad-hoc special case for those commands? >> things like `this-command' are always brittle, so while it's often >> unavoidable, I want to make sure it really is so. > Avoids "flicker" in the minibuffer. Hmm... there shouldn't be any such visible flicker, even if we delete-overlay and then re-add the overlay in post-command-hook (because there shouldn't be any redisplay between the two). Do you have a test case showing this problem? Also, why include minibuffer-force-complete-and-exit in this list, since it will exit the minibuffer so the post-command-hook won't be run, so there won't be flicker anyway. >> I see why this is often a good idea, but I'd be interested to hear >> concrete use-cases where this was useful/needed. The reason for it is >> that the default ordering (shortest first) already should give you the >> "exact but not unique is first". And the other ordering rule (prefer >> elements from the history) sounds just as valid as the one you suggest, >> so I'm not sure why yours should take precedence. > While in buffer, history takes precedence over the length rule. It is > possible that the exact match gets stacked deep in the summary. > Also when common substring is stripped, the exact match looks weird - it > becomes an empty string and is easily recognized when it is a first > item. Right, which is why icomplete.el already displays it first, no matter where it is in the list. >> Furthermore, the "exact but not unique first" is actually kind of >> useless in the sense that the user can already just hit RET to get that >> one, so she doesn't need much more help to access it. >>> + ;; Delete duplicates. >>> + (delete-dups all)) > The code merely moves the following condition in icomplete.el to it's > rightful place which is sorted completion itself. Then I'd rather not do it. If you have a specific use-case where that solves a misbehavior, then we can discuss it, but otherwise I'd rather focus on the main task at hand: incremental improvements to bring icomplete.el a bit closer to ido.el. Stefan