Add customizable variables for cheatsheet #3690

katomuso · 2024-05-31T10:18:11Z

Add multiple customizable variables to control different aspects of cider-cheatsheet and cider-cheatsheet-select behavior.

katomuso · 2024-05-31T10:28:45Z

@bbatsov Can I be added to the cider-cheatsheet.el package authors?

bbatsov · 2024-06-02T05:38:51Z

cider-cheatsheet.el

+
+(defcustom cider-cheatsheet-auto-select-buffer t
+  "Whether to auto-select the cheatsheet popup buffer."
+  :type 'boolean)


You should also add :package-version properties to all defcustoms (saying 1.15.0 as the version).

bbatsov · 2024-06-02T05:40:32Z

cider-cheatsheet.el

+                 (const :tag "ClojureDocs" cider-clojuredocs-lookup)
+                 function))
+
+(defcustom cider-cheatsheet-select-path nil


Probably it'd better for this to be an option taking some symbol (the name of the type of behavior to use) instead of a boolean - it would make the code more flexible if we decide to add different options down the road.

I think that it's a good idea to create a variable like cider-cheatsheet-select-type or cider-cheatsheet-select-display, which accepts symbols like multi (default) and path. However, one small consequence of this approach is that we won't be able to use cider-cheatsheet-select with a prefix argument anymore as it doesn’t make sense to negate the value of this variable as it did with cider-cheatsheet-select-path, which accepted a boolean. Will this be okay?

Actually, we can do something similar. We just need to check the exact prefix argument value and use the value of cider-cheatsheet-select-type accordingly, as it is done in cider-jack-in-universal, right?

That's one option, another would be to just drop the prefix argument and rely only on the defcustom.

I think it will be better to drop the prefix argument as most users will probably always use one way or the other.

bbatsov · 2024-06-02T05:42:38Z

cider-cheatsheet.el

+  "Whether to auto-select the cheatsheet popup buffer."
+  :type 'boolean)
+
+(defcustom cider-cheatsheet-doc-function #'cider-doc-lookup


Maybe use a more generic name like cider-cheatsheet-(default)-action-function? And obviously in the buffer view we can also have more actions bound to different keybindings (the default could be the one triggered by RET)

I'm not exactly sure what you mean. Maybe you can point me to some place where such logic is already implemented, so I can understand it better? I think some examples of values for such a variable will also be helpful.

If I understand correctly, you're suggesting renaming it to a more generic name because it's not guaranteed that by selecting a var, the user wants to show documentation. Did I understand that correctly?

Yeah, exactly. You can check the namespace navigator or apropos where you can trigger different actions in the buffer displaying the results (e.g. you can show the docs or go to the definition for some var), and there's some default action triggered by RET. I'm not sure if that makes much sense here, but it's another case where we are dealing with var names, so we might aim for some consistency.

I agree, and it actually looks more generic, which is a good thing.

bbatsov · 2024-06-02T05:44:30Z

cider-cheatsheet.el

+such as '/' and '&'."
+  :type 'string)
+
+(defcustom cider-cheatsheet-indentation-function


I'm not quite certain that many people will want to modify this. Do you have any particular use-case in mind?

I thought that, since it is related to UI, some users might want to modify how it looks. However, I agree that most users won't do this, and it is actually not that safe to modify that variable, as it can break cider-cheatsheet-select. So, I think it would be a good idea to move it to a constant.

Oh, I assumed you meant cider-cheatsheet-select-path-separator. If it is cider-cheatsheet-indentation-function, then some people prefer 2 spaces, others 4 spaces, tabs, or other indication of indentation. This use-case is what I had in mind when adding this as a customizable variable.

Seems like premature optimization to me. I usually wait for the users to request some config options if I have doubts that there would be demand for them.

I think that's a good approach, so for now, they will be removed.

bbatsov · 2024-06-02T05:44:48Z

@bbatsov Can I be added to the cider-cheatsheet.el package authors?

Sure.

katomuso · 2024-06-02T11:35:36Z

@bbatsov Can I be added to the cider-cheatsheet.el package authors?

Sure.

Thank you! I'll update the package commentary in a separate PR after this one is finished.

katomuso · 2024-06-02T22:12:53Z

I've made all the changes as discussed in the review:

Renamed cider-cheatsheet-doc-function to cider-cheatsheet-default-action-function and tweaked docstring
Renamed cider-cheatsheet-select-path to cider-cheatsheet-select-type, tweaked docstring and removed prefix argument for cider-cheatsheet-select
Removed cider-cheatsheet-select-path-separator and cider-cheatsheet-indentation-function

bbatsov · 2024-06-02T22:20:24Z

cider-cheatsheet.el

+If `path`, then represent each completion candidate as a full path to a var."
+  :type '(choice (const multi)
+                 (const path))
+  :package-version '(cider . "0.15.0"))


My bad, fixed!

bbatsov · 2024-06-02T22:23:09Z

cider-cheatsheet.el

+                 function)
+  :package-version '(cider . "0.15.0"))
+
+(defcustom cider-cheatsheet-select-type 'multi


Let me think a bit about the names multi and path, because I'm not sure those are optimal here.

Sure. I think so as well, but it was tough to come up with something better.

Some alternatives I came up with are multi-step and full-path, or nested and flat.

Well, come to think of it, maybe just having a prefix argument for cider-cheatsheet-select will be enough for now.

Fair enough. :-)

Okay, I've pushed the change and updated the docs in #3695.

bbatsov · 2024-06-03T13:15:50Z

Thanks!

katomuso mentioned this pull request Jun 1, 2024

Add a section on cheatsheet to the docs #3695

Merged

bbatsov reviewed Jun 2, 2024

View reviewed changes

katomuso force-pushed the cheatsheet-custom branch from 3dbd140 to 162ff4b Compare June 2, 2024 10:28

katomuso force-pushed the cheatsheet-custom branch from 162ff4b to d43d89e Compare June 2, 2024 22:04

bbatsov reviewed Jun 2, 2024

View reviewed changes

katomuso force-pushed the cheatsheet-custom branch 2 times, most recently from 28ef58e to 1455194 Compare June 2, 2024 22:30

katomuso added 4 commits June 3, 2024 11:20

Move cheatsheet buffer name to cider-cheatsheet-buffer constant

0ce4776

Add customization group cider-cheatsheet

d43a0da

Add customizable variable cider-cheatsheet-auto-select-buffer

a379f96

Add customizable variable cider-cheatsheet-default-action-function

aec4560

katomuso force-pushed the cheatsheet-custom branch from 1455194 to aec4560 Compare June 3, 2024 11:21

bbatsov merged commit 78e4a1c into clojure-emacs:master Jun 3, 2024
39 checks passed

katomuso deleted the cheatsheet-custom branch June 3, 2024 13:25

katomuso restored the cheatsheet-custom branch June 3, 2024 17:29

katomuso deleted the cheatsheet-custom branch June 3, 2024 17:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add customizable variables for cheatsheet #3690

Add customizable variables for cheatsheet #3690

katomuso commented May 31, 2024

katomuso commented May 31, 2024

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

katomuso Jun 2, 2024 •

edited

Loading

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

katomuso Jun 2, 2024

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

katomuso Jun 2, 2024

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

bbatsov commented Jun 2, 2024

katomuso commented Jun 2, 2024

katomuso commented Jun 2, 2024

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

bbatsov Jun 2, 2024

katomuso Jun 2, 2024

katomuso Jun 2, 2024 •

edited

Loading

katomuso Jun 2, 2024

bbatsov Jun 3, 2024

katomuso Jun 3, 2024

bbatsov commented Jun 3, 2024

Add customizable variables for cheatsheet #3690

Add customizable variables for cheatsheet #3690

Conversation

katomuso commented May 31, 2024

katomuso commented May 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katomuso Jun 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbatsov commented Jun 2, 2024

katomuso commented Jun 2, 2024

katomuso commented Jun 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katomuso Jun 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbatsov commented Jun 3, 2024

katomuso Jun 2, 2024 •

edited

Loading

katomuso Jun 2, 2024 •

edited

Loading