|Subject:||Re: Ibuffer improvements: filtering, documentation, bug fix, tests|
|Date:||Wed, 30 Nov 2016 22:10:58 -0500|
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 theVery good. That would be helpful. Thank you.
saved filters fix, but I will do that tomorrow and submit
appropriate patches and bug reports.
The buffer name filter you suggest is already available asRight! I totally overlooked that.
ibuffer-filter-by-name (/ n).
1. Add a `ibuffer-filter-by-visiting-fi+(define-ibuffer-filter visiting-file
le' (/ v) that selects
buffers that are visiting a 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:
"Limit current view to buffers that are visiting a file."
(:description "visiting a file"
(with-current-buffer buf buffer-file-name))
`ibuffer-filter-by-directory' and changed the functionality so thatI am wondering if it's worth to have the `ibuffer-filter-by-directory'
in a file buffer it matches against the file's path but in a
non-file buffer matches against default-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:
"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): ")))
(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 nameI 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 haveOpps! I didn't notice this. Thanks.
I think the new bindings are highly mnemonic and will happily advocateOnce we are happy with the changes we might ask opinion to other
for them. But the need for consensus makes total sense.
I felt that the change I made keeps the mnemonic strong.
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.
Thank you. They like quite verbose for NEWS entries. We just needYou might want to write NEWS entry for the new features.Done, and included in this commit/patch.
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',
le'; bound respectively
to '/b', '/.', '//', '/*', '/i' and '/v'.
*** Two new commands 'ibuffer-filter-chosen-by-comp
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>'
You are right, it seems there is a bug.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.
I will pull out the saved filter changes and submit aThat will be very useful. Thank you.
formal bug report with patches for the two approaches, making
the other ibuffer changes independent.
Description: Text Data
|[Prev in Thread]||Current Thread||[Next in Thread]|