[inspector] Add configuration for max-nested-depth and spacious #3664
Conversation
|
Looks good. A cider-nrepl artifact is being built. I'll bump it here then (please don't merge this before that) |
|
Done, you can merge master in |
alexander-yakushev
force-pushed
the
inspector
branch
from
34786fa to
5e423d5
Compare
|
Thank you! I added a CHANGELOG commit, or is that unnecessary? |
alexander-yakushev
force-pushed
the
inspector
branch
2 times, most recently
from
6cca5ca to
c689663
Compare
|
I changed |
alexander-yakushev
force-pushed
the
inspector
branch
from
c689663 to
fc7d0ce
Compare
|
Fair enough. I asked about this before seeing your comment. I'd suggest to mention the changes in this PR in the changelog as well and it'd be good if the new defcustom is also mentioned in the inspector's docs, so more people would actually discover it. |
alexander-yakushev
force-pushed
the
inspector
branch
from
fc7d0ce to
a8ce01e
Compare
|
Ping 🙂 |
|
@alexander-yakushev Did you see my previous small remarks? I see there no changes after writing them (e.g. the incorrect CIDER version in one defcustom) |
|
Yep, I addressed them right after. |
cider-inspector.el
Outdated
| "Default level of nesting for collections to display before truncating. | ||
| The max depth can be also changed interactively within the inspector." | ||
| :type '(integer :tag "Max nested collection depth" 5) | ||
| :package-version '(cider . "1.1.4")) |
There was a problem hiding this comment.
I still see the wrong version here.
There was a problem hiding this comment.
Right, this is my bad.
| :package-version '(cider . "1.1.4")) | ||
|
|
||
| (defvar cider-inspector-spacious-collections nil | ||
| "Controls whether the inspector renders values in collections spaciously.") |
There was a problem hiding this comment.
Maybe explain a bit more what "spaciously" means in this context. If I didn't know what this controls I'd have a hard time figuring it out.
Btw, why did you decide to make this a defvar instead of defcusom? I doubt anyone was particularly attached to the extra spaces, but defcustoms are easier to discover.
There was a problem hiding this comment.
I saw your other comment, forget about this.
| @@ -68,6 +68,10 @@ You'll have access to additional keybindings in the inspector buffer | |||
| | `cider-inspector-set-max-coll-size` | |||
| | Set a new maximum size above which nested collections are truncated | |||
|
|
|||
| | kbd:[C] | |||
There was a problem hiding this comment.
I think it'd better to pick some free lowercase letter for this. (e.g. d)
There was a problem hiding this comment.
I chose a capital letter intentionally to not take away a letter from the binding space. I don't consider this customization to be used often or ever. We'll probably change the default some time down the road I expect this variable will be changed dynamically in some unique cases, but otherwise, it's a meh variable to customize.
There was a problem hiding this comment.
Besides, there is certain synergy in changing length (*print-length*) and depth (*print-depth*), so they can be neighbours on the same letter, even if the letter itself does not relate to depth much.
| inspector buffer using the `s`, `c`, and `a` keybindings. | ||
| `cider-inspector-page-size`, `cider-inspector-max-coll-size`, | ||
| `cider-inspector-max-nested-depth`, and `cider-inspector-max-atom-length`. The | ||
| values can be adjusted for the current inspector buffer using the `s`, `c`, and |
There was a problem hiding this comment.
You didn't mention the new keybinding here. I wonder if it's a good idea to repeat here the data from the table above, though. Perhaps this sentence can be removed or made more generic to require fewer updates.
|
Hmm, perhaps I didn't press the button "finish review" then. 😅 |
alexander-yakushev
force-pushed
the
inspector
branch
from
a8ce01e to
5a0f08d
Compare
|
Thanks and sorry about the delay here! |
The PR doesn't depend on
cider-nreplbeing the latest version, but a release of it would be appreciated before merging. CIDER versions in the customs are approximated.