-
-
Notifications
You must be signed in to change notification settings - Fork 645
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
Add an alternative way to display cider-cheatsheet-select
#3686
Add an alternative way to display cider-cheatsheet-select
#3686
Conversation
629938c
to
9ea72ba
Compare
(let* ((vars (seq-mapcat #'cider-cheatsheet--expand-vars hierarchy)) | ||
(var (completing-read "Select var: " vars))) | ||
(cider-doc-lookup var)))) | ||
(defun cider-cheatsheet-select (&optional flat) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this can be a defcustom instead and you can use the prefix arg to invert whatever the value of the defcustom is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! I've seen such a pattern a few times, but haven't thought about it this time. I planned to think about customizable variables later, so I will keep this in mind.
Nice improvement - i definitely like it. |
I wonder why the lint check failed because |
Oh, I found it. It has to do with byte compilation and the |
You just need to require |
9ea72ba
to
30a212a
Compare
Hmm, it seems that the error is still present, as |
Ah, my bad. Then it's very weird, as |
Hmm, I see that the function is actually named |
@@ -32,6 +32,7 @@ | |||
(require 'cl-lib) | |||
(require 'map) | |||
(require 'seq) | |||
(require 'subr-x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In light of my last comment you can remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I actually use string-join
from subr-x
, I think it will be okay to be explicit about it by requiring it.
cider-cheatsheet.el
Outdated
(let* ((hierarchy (cider-cheatsheet--flatten-hierarchy cider-cheatsheet-hierarchy)) | ||
(paths (mapcar (lambda (sections) (string-join sections " > ")) hierarchy)) | ||
(path (completing-read "Select path: " paths)) | ||
(var (car (last (string-split path " > "))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually split-string
. See https://www.gnu.org/software/emacs/manual/html_node/elisp/Creating-Strings.html
No idea how this ever worked for you, if the name of the function is wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this is an alias. I guess that may be the reason it can't find it during byte compilation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also seems this alias was only added in Emacs 29.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, changing the name fixed it. It still seems weird, but I'll be more cautious with aliases from now on :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also seems this alias was only added in Emacs 29.
I see, so that's the reason. Thanks for helping me figure it out.
30a212a
to
a4d16c0
Compare
So now that all the checks are passing, can I add a new entry to the changelog so we can merge the change? I'll add customizable variables in the future as I am still not sure about some of the details. |
Sure.
…On Wed, May 29, 2024, at 12:47 PM, Kato Muso wrote:
So now that all the checks are passing, can I add a new entry to the changelog so we can merge the change? I'll add customizable variables in the future as I am still not sure about some of the details.
—
Reply to this email directly, view it on GitHub <#3686 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZLSSHRK6MY4MLFXJ4MVDZEWW5FAVCNFSM6AAAAABIOL3A7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZXGEYTINRZGM>.
You are receiving this because you commented.Message ID: ***@***.***>
|
Thanks! |
Instead of having the multi-step selection process, which is the default
cider-cheatsheet-select
behavior, we represent each candidate as a full path to a var whencider-cheatsheet-select
is called with a prefix argument. This can be handy with fuzzy completion style and vertical candidates display, as discussed in #3678.Here's how the default multi-step selection process looks when you need to select every single section before choosing a var:
In contrast, when
cider-cheatsheet-select
is called with a prefix argument, each candidate is a full path to a var: