[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Ibuffer improvements: filtering, documentation, bug fix, tests

From: Christopher Genovese
Subject: Re: Ibuffer improvements: filtering, documentation, bug fix, tests
Date: Wed, 30 Nov 2016 22:10:58 -0500


  Sorry it took so long to get this to you; it's been a crazy week.
I've attached a patch file with all the changes we have discussed
(except one, see below) to the code, change logs, and NEWS
and with the saved filter bug changes completely removed as you
requested.  This is all up to date with the current master. I think
the patch has everything, but let me know if I missed something.

  The one change I did not make is to the ibuffer-filter-by-directory
filter as discussed in your most recent note.  My intent with that
one is to make it easy to filter on the directory component of a
filename (or of the buffer) without patterns in the filename interfering.
I don't see the advantage of conflating this with dired mode
or adding a prefix, especially as the two functionalities are
not intuitively comparable.  I'm happy to discuss this, but
I left it in the way I prefer for the time being.

 Thanks again for your help, suggestions, and patience.

  Regards, Chris

On Sat, Nov 26, 2016 at 5:53 AM, Tino Calancha <address@hidden> wrote:

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)
          (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 '/&'

*** 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 '/<up>', '/S-<up>'
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.


Attachment: ibuffer-and-filters-revised.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]