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

Conversation

justbur
Copy link
Contributor

@justbur justbur commented Jun 29, 2018

Filter out vars without custom-type properties.

Filter out vars without custom-type properties.
(lambda (var)
(get (intern var) 'custom-type))
vars)
vars))
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.

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

@justbur justbur Jun 29, 2018

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

@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 Can you please elaborate?

Copy link
Contributor Author

@justbur justbur Jun 29, 2018

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

@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.

Oh, seems to be #1158.

@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Jul 3, 2018

Thanks.

basil-conto added a commit to basil-conto/swiper that referenced this issue Aug 7, 2018
counsel.el (counsel-set-variable):
Use custom-variable-p instead of inspecting the custom-type
property, which is only set when a defcustom has a :type.

Re: abo-abo#1643
abo-abo pushed a commit that referenced this issue Aug 8, 2018
counsel.el (counsel-set-variable):
Use custom-variable-p instead of inspecting the custom-type
property, which is only set when a defcustom has a :type.

Re: #1643
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

3 participants