Use new Orchard #869
Use new Orchard #869
Conversation
| "inspect-next-page" next-page-reply | ||
| "inspect-prev-page" prev-page-reply | ||
| "inspect-set-page-size" set-page-size-reply | ||
| "inspect-set-max-atom-length" set-max-atom-length-reply | ||
| "inspect-set-max-value-length" set-max-value-length-reply |
There was a problem hiding this comment.
While we can keep this, it would be also much welcome to have an options map so that users can simply have a defcustom.
This way we have fewer calls / moving pieces.
Existing example:
There was a problem hiding this comment.
I agree it feels excessive to be able to customize each of those separately on the fly. Being able to set it up in one place would be better. At the same time, I don't know if a single set of options will fit everything. For example, it feels like the debugger can use stricter limitations than inspector (because debugger shows values inline with the code and inspector has a dedicated buffer).
I also think that different options have different utility in being configurable. E.g., customizing page-size on the fly is awesome. Being able to change coll-length and coll-depth on the fly in the inspector can be useful – if you need to see just a couple extra items or levels. But changing atom-length and total-length is of questionable benefit to me.
There was a problem hiding this comment.
Yeah I don't oppose this op, and similarly, not everything has to be exposed as an option.
Simply trying to make sure that we can get the most useful options easily conveyed from cider.el to the Inspector.
There was a problem hiding this comment.
Here's what I could do:
- Expose all five of these parameters as defcustoms in CIDER. Let the user customize these defaults as they wish.
- Keep page-size and coll-length dynamic ops as is. Add a new coll-depth op, bind it to
C(shift-c) in CIDER inspector to not hijack another letter for a pretty useless command (cis currently for changing coll-length). - Drop atom-length op from cider-nrepl. Make atom-length and value-length only changeable through defcustoms.
Thoughts?
There was a problem hiding this comment.
I'd do 1 and 2. 3, I'd try really hard to avoid breaking changes.
Perhaps clojure-emacs/cider#2960 would be a good read - starting from that discussion (and increasingly since) we've been trying to commit to avoiding breakage.
|
Re clojure-emacs/orchard#227 (comment) , maybe we can set that default here in advance |
alexander-yakushev
force-pushed
the
new-orchard
branch
6 times, most recently
from
1c20a44
to
8c1d6df
Compare
While there is a default of 50000 (soon to be 10000) in |
alexander-yakushev
force-pushed
the
new-orchard
branch
from
8c1d6df
to
7b570de
Compare
alexander-yakushev
force-pushed
the
new-orchard
branch
2 times, most recently
from
a2f21d6
to
480cca1
Compare
|
Ready now. |
| @@ -364,6 +361,11 @@ if applicable, and re-render the updated value." | |||
| :requires {"max-coll-size" "New collection size." | |||
| "session" "The current session"} | |||
| :returns inspector-returns} | |||
| "inspect-set-max-nested-depth" | |||
There was a problem hiding this comment.
Good to specifically add to the changelog
alexander-yakushev
force-pushed
the
new-orchard
branch
from
480cca1
to
5c6eebb
Compare
|
I've installed this. First impression is of course good. The inspector is my "repl" so to speak so there will be a fair bit of usage over the next days. |
Update Orchard to the latest version. This gets all latest inspector updates and new
orchard.printnamespace.Adapt inspector middleware to updated
orchard.inspectUse new printing in cider debugger
You've added tests to cover your change(s)
You've updated the README
Middleware documentation is up to date
cider.nreplns which has all middleware documentation.lein docsafterwards, and commit the results.