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 (counsel-set-variable): Add prefix arg behavior. #1643

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions counsel.el
Original file line number Diff line number Diff line change
Expand Up @@ -647,10 +647,19 @@ When the selected variable is a `defcustom' with the type boolean
or radio, offer completion of all possible values.

Otherwise, offer a variant of `eval-expression', with the initial
input corresponding to the chosen variable."
input corresponding to the chosen variable.

With a prefix arg, restrict list to variables defined using
`defcustom'."
(interactive (list (intern
(ivy-read "Variable: "
(counsel-variable-list)
(let ((vars (counsel-variable-list)))
(if current-prefix-arg
(cl-remove-if-not
(lambda (var)
(get (intern var) 'custom-type))
vars)
vars))
Copy link
Collaborator

Choose a reason for hiding this comment

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

AKA

(ivy-read "Set variable: " (counsel-variable-list)
          :predicate (and current-prefix-arg
                          (lambda (varname)
                            (get (intern varname) 'custom-type)))
          :history 'counsel-set-variable-history
          :preselect (ivy-thing-at-point)
          :sort t
          :caller 'counsel-set-variable)

(Any reason :sort and :caller weren't already specified?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@basil-conto That version is nicer but it doesn't work for me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@justbur Can you please elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@basil-conto Yes, sorry. With your version a prefix arg does not seem to have any affect on the list of candidates, while my initial version did eliminate candidates in my tests. I'm not sure why

Copy link
Collaborator

@basil-conto basil-conto Jun 29, 2018

Choose a reason for hiding this comment

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

@justbur Looks like there is a bug with Ivy where the :predicate is never called for some reason. I won't be able to investigate until tomorrow, so unless you care to investigate before then, I suspect @abo-abo will just merge as is.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, seems to be #1158.

:preselect (ivy-thing-at-point)
:history 'counsel-set-variable-history))))
(let ((doc (and (require 'cus-edit)
Expand Down