Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

counsel.el: Add command counsel-ibuffer #1467

Closed
wants to merge 4 commits into from

Conversation

fabacino
Copy link
Contributor

@fabacino fabacino commented Feb 20, 2018

Simple counsel version of ibuffer.
Credit for the idea belongs to @joedicastro.

Deprecates #943

counsel.el Outdated
@@ -4476,6 +4476,77 @@ replacements. "
(when (fboundp 'advice-remove)
(advice-remove #'describe-bindings #'counsel-descbinds))))

;;** `counsel-ibuffer'
(defvar counsel-ibuffer-buffer-name nil
Copy link
Collaborator

@basil-conto basil-conto Feb 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any merits/downsides to making this a defcustom?

Copy link
Contributor Author

@fabacino fabacino Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an internal variable and gets overwritten as soon as counsel-ibuffer is called. The name of the ibuffer you want to use can be specified through the argument name when calling counsel-ibuffer.
I admit the doc string is misleading, I will also add an additional - to the variable name to make it clear that it's meant for internal use only.

Copy link
Collaborator

@basil-conto basil-conto Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabacino OK, thanks for clarifying, I was just curious.

counsel.el Outdated
;; by comparing line numbers.
(let ((prev-lineno 0)
lineno)
(while (> (setq lineno (line-number-at-pos)) prev-lineno)
Copy link
Collaborator

@basil-conto basil-conto Feb 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more efficient to just check (point) instead of line number?

Copy link
Contributor Author

@fabacino fabacino Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I refactored the iteration to compare points.

counsel.el Outdated
(when (buffer-live-p current-buf)
(push
(cons
(substring-no-properties (thing-at-point 'line) 0 -1)
Copy link
Collaborator

@basil-conto basil-conto Feb 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a.k.a. (buffer-substring-no-properties (line-beginning-position) (line-end-position))?

@mookid
Copy link
Contributor

@mookid mookid commented Feb 20, 2018

The open vanilla ibuffer action fails here with make plain:

funcall: Wrong number of arguments: ((org-odd-levels-only shr-color-html-colors-alist shr-color-visible-luminance-min company-common company-point company-candidates hydra-curr-body-fn eshell-history-ring imenu-auto-rescan-maxout imenu-auto-rescan ...) nil "Switch to vanilla ibuffer." (switch-to-buffer counsel-ibuffer-buffer-name)), 1

An easy fix:

--- a/counsel.el
+++ b/counsel.el
@@ -4538,7 +4538,7 @@ NAME specifies the name of the buffer (defaults to \"*Ibuffer*\")."
   "Switch to buffer of candidate X in other window."
   (switch-to-buffer-other-window (cdr x)))
 
-(defun counsel-ibuffer-visit-vanilla-ibuffer ()
+(defun counsel-ibuffer-visit-vanilla-ibuffer (_)
   "Switch to vanilla ibuffer."
   (switch-to-buffer counsel-ibuffer-buffer-name))

@fabacino fabacino force-pushed the feature/counsel-ibuffer branch from 1799310 to 82da2c2 Compare Feb 21, 2018
fabacino added 2 commits Feb 21, 2018
…ound

Instead of comparing line numbers, we remember the point of the first
entry and abort the iteration as soon as this point is encountered for
the second time.
@fabacino
Copy link
Contributor Author

@fabacino fabacino commented Feb 21, 2018

@basil-conto @mookid Thank you both for your suggestions. I've updated the PR accordingly.

@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Feb 21, 2018

I get infloop with this setting in my config:

(setq ibuffer-movement-cycle nil)

@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Feb 21, 2018

Merged, thanks all.

@fabacino fabacino deleted the feature/counsel-ibuffer branch Feb 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants