From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Tino Calancha Newsgroups: gmane.emacs.devel Subject: Re: Ibuffer improvements: filtering, documentation, bug fix, tests Date: Sat, 26 Nov 2016 19:53:10 +0900 (JST) Message-ID: References: <877f7zhg79.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; format=flowed; charset=US-ASCII X-Trace: blaine.gmane.org 1480157646 29464 195.159.176.226 (26 Nov 2016 10:54:06 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 26 Nov 2016 10:54:06 +0000 (UTC) User-Agent: Alpine 2.20 (DEB 67 2015-01-07) Cc: emacs-devel , Tino Calancha To: Christopher Genovese Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Nov 26 11:53:58 2016 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cAacG-0006Fd-V2 for ged-emacs-devel@m.gmane.org; Sat, 26 Nov 2016 11:53:57 +0100 Original-Received: from localhost ([::1]:49986 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cAacK-0006W0-De for ged-emacs-devel@m.gmane.org; Sat, 26 Nov 2016 05:54:00 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:56591) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cAabf-0006Vv-Dn for emacs-devel@gnu.org; Sat, 26 Nov 2016 05:53:20 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cAabc-0000sE-BK for emacs-devel@gnu.org; Sat, 26 Nov 2016 05:53:19 -0500 Original-Received: from mail-pf0-x229.google.com ([2607:f8b0:400e:c00::229]:33309) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cAabc-0000s9-10 for emacs-devel@gnu.org; Sat, 26 Nov 2016 05:53:16 -0500 Original-Received: by mail-pf0-x229.google.com with SMTP id d2so17999450pfd.0 for ; Sat, 26 Nov 2016 02:53:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:date:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version; bh=gV7LW9w5Vgv6yRB3fl4SkIeJf/n+0JVO6LIjFMzr8gE=; b=zo6nnj06NbK+YGuEroW3RcFurIWxejPhcESxsoTZSMrD/FjfJGumBYlgIEbITNgu2c oLQhy4+QaWNATjqZDnYfw+S6UJNyBtg8hSWSFGxWZykxJZXm3565rFjyxKH80704NIhE pz7KQP8B3wyQ5R/IjeLz0+wld+EjRWM3nsoj16xFpUa8PSF+iG12NNyUUB22TIDrpNc8 dZz2xRvh9IbhsQb3Q2/4Z3vfF8yBaSWGToaHTLXyoRdNkpiWr3k6CmZli44ulzSsdwVP hTnkO4T229zoBYCLHxbxAVPRbcKoKKdGV0EHMwhTJFlNdkHnPfuiAN8J37mpUjGm8CXB mxjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=gV7LW9w5Vgv6yRB3fl4SkIeJf/n+0JVO6LIjFMzr8gE=; b=AzHCkGakeFWrQYFdtvYTHHImY7S1w1FwKP9yH9PxGdnRE5ILcO1SbpzwR0YoHojwTR msoz2W7DjMe5BObn+kcM49jqbS//lzkP6EDxXdHbjg8zn8WfCI5ycOEE/fsRUnvxWWHB zJA/3NxgGDFxHtpJQW1SWqOCL+GYJPLIZhBjzJrjfmp8l1xElAjdiLoxB4+xB/HCMKQS xtjRRlBO1CmtweSgwdtpd60QjB0gQuJiFjOOP+KMQy5UGolTh/9m1z6H71tvAnh0VwGm G42ixB5g+B+Oo/e7hXiTjBcg63PsaGKG/safIAW3uvYM9gikey6oochXIasYCE0/X+qX hPcQ== X-Gm-Message-State: AKaTC034Jj0xWvL9IojMmCijyQPKLYfAxlHlpiPwy/7Afa4RPkK6yGwNrMLyKaNzbskK6w== X-Received: by 10.84.130.5 with SMTP id 5mr11653924plc.69.1480157594458; Sat, 26 Nov 2016 02:53:14 -0800 (PST) Original-Received: from calancha-pc (57.92.100.220.dy.bbexcite.jp. [220.100.92.57]) by smtp.gmail.com with ESMTPSA id 1sm46476092pgp.1.2016.11.26.02.53.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 26 Nov 2016 02:53:13 -0800 (PST) X-Google-Original-From: Tino Calancha X-X-Sender: calancha@calancha-pc In-Reply-To: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:400e:c00::229 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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" Xref: news.gmane.org gmane.emacs.devel:209613 Archived-At: Hi Christopher, thank you very much for your time working on this! I have checked your newest changes and i got a few additional comments. See below. >I haven't had a chance yet to split the commit to isolate the >saved filters fix, but I will do that tomorrow and submit >appropriate patches and bug reports. Very good. That would be helpful. Thank you. >The buffer name filter you suggest is already available as >ibuffer-filter-by-name (/ n). Right! I totally overlooked that. > 1. Add a `ibuffer-filter-by-visiting-file' (/ v) that selects > buffers that are visiting a file. +(define-ibuffer-filter visiting-file + "Limit current view to buffers that are visiting a file. +This includes buffers visiting a directory in dired." I wouldn't include Dired buffers here. In Emacs a buffer visiting a file means a non-directory file. For instance, look doc string of `buffer-file-name'. I suggest instead: (define-ibuffer-filter visiting-file "Limit current view to buffers that are visiting a file." (:description "visiting a file" :reader nil) (with-current-buffer buf buffer-file-name)) > `ibuffer-filter-by-directory' and changed the functionality so that > in a file buffer it matches against the file's path but in a > non-file buffer matches against default-directory. I am wondering if it's worth to have the `ibuffer-filter-by-directory' in the way you are proposing. I guess `ibuffer-filter-by-filename' would suffice most of the times. In the other hand we have `ibuffer-mark-dired-buffers' bound to '*/' that is handy. We might want `ibuffer-filter-by-directory' to do the symmetric thing: that is, to filter buffers in Dired mode, i.e., like a shortcut for '/ m' dired-mode RET. Alternatively we could accept a prefix in this command: 1) Without prefix, just filter buffers in Dired mode. 2) With a prefix, behave as you wish, as follows: (define-ibuffer-filter directory "Limit current view to Dired buffers. With prefix argument prompt for a regexp and show just those buffers with their directory matching that regexp. For a buffer associated with file '/a/b/c.d', this matches against '/a/b'. For a buffer not associated with a file, this matches against the value of `default-directory' in that buffer." (:description "directory name" :reader (and current-prefix-arg (read-from-minibuffer "Filter by directory name (regex): "))) (with-current-buffer buf (if qualifier (let ((dirname (ibuffer-aif (ibuffer-buffer-file-name) (file-name-directory it) default-directory))) (and dirname (string-match qualifier dirname))) (eq major-mode 'dired-mode)))) This means the command do a different thing if we provide the prefix. I don't know what approach is more useful. Does 1) or 2) has sense for you? >3. Keep the `ibuffer-filter-by-basename', making the name I saw you bound this command to '/ b'. Good! I find easier to remember and type '/ b' than '/ F'. >(Note: '/ d' is already bound to ibuffer-decompose-filter or I would have >used it. Opps! I didn't notice this. Thanks. >I think the new bindings are highly mnemonic and will happily advocate >for them. But the need for consensus makes total sense. >I felt that the change I made keeps the mnemonic strong. Once we are happy with the changes we might ask opinion to other colleagues in Emacs-dev about what to do with the bindings. >> Your commit message don't follow the Emacs standards. > I've fixed this on the new commit. >* lisp/ibuf-ext.el: added paragraph to file commentary, along ^^^^^ >(ibuffer-saved-filters): clarified documentation, ^^^^^^^^^ Please, start sentences with upper case. >> You might want to write NEWS entry for the new features. >Done, and included in this commit/patch. Thank you. They like quite verbose for NEWS entries. We just need to announce the changes. It's OK to group all new commands in the same entry. Let's ignore the entry for the bug fix. We will back to that issue once we open the bug report. I suggest the following shorter entries: --- *** New filter commands `ibuffer-filter-by-basename', `ibuffer-filter-by-file-extension', `ibuffer-filter-by-directory', `ibuffer-filter-by-starred-name', `ibuffer-filter-by-modified' and `ibuffer-filter-by-visiting-file'; bound respectively to '/b', '/.', '//', '/*', '/i' and '/v'. --- *** Two new commands 'ibuffer-filter-chosen-by-completion' and `ibuffer-and-filter'; bound to '/ TAB' and '/&' respectively. --- *** The key binding for `ibuffer-filter-disable' has being changed to '/DEL'; the commands `ibuffer-pop-filter', `ibuffer-pop-filter-group' and `ibuffer-or-filter' have the alternative bindings '/', '/S-' and '/|'. >> Could you create a receipt where the bug cause an actual failure? >As you can see, the existing entry "gnus" breaks the expected format. >So to be more precise than I was earlier: In addition to the unnecessary >nesting level, this breaks anytime you save to an existing filter. You are right, it seems there is a bug. >I will pull out the saved filter changes and submit a >formal bug report with patches for the two approaches, making >the other ibuffer changes independent. That will be very useful. Thank you. Regards, Tino