[inspect] Rework view-mode toggling #343
Conversation
alexander-yakushev
force-pushed
the
view-mode-toggle
branch
5 times, most recently
from
88fe3f7 to
86be9ee
Compare
| (pre-ex (view-mode-supported? inspector mode)) | ||
| (inspect-render (assoc inspector :view-mode mode))) | ||
|
|
||
| (defn toggle-view-mode |
There was a problem hiding this comment.
Toggle usually implies that something is binary. I think here it's more like "cycle".
There was a problem hiding this comment.
I agree but that ship kinda sailed since we have inspect-toggle-view-mode nrepl op, and it makes sense to keep things consistent. Wrt renaming it everywhere, I don't think it warrants a full rename right now.
There was a problem hiding this comment.
Let's put it off until the big op namespacing refactor.
|
The proposed changes look good to me. |
alexander-yakushev
force-pushed
the
view-mode-toggle
branch
from
86be9ee to
17da143
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the inspector’s view-mode system to support cycling through available modes and better discovery.
- Introduces a multimethod
view-mode-supported?and replaces the staticsupported-view-modesset - Adds
set-view-mode,toggle-view-mode, and updatesrender-view-modeto show mode toggles (including pretty-print) - Updates tests to match the new display format and adds coverage for
toggle-view-mode
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/orchard/inspect.clj | Replaced static mode set with multimethods, added toggle logic, updated rendering of “View mode” section |
| test/orchard/inspect_test.clj | Updated many expected outputs to remove extra newlines, added tests for toggle-view-mode |
| CHANGELOG.md | Added entry for rework of view-mode toggling |
Comments suppressed due to low confidence (2)
src/orchard/inspect.clj:291
- [nitpick] Consider renaming
view-mode-supported?tosupports-view-mode?to match predicate naming conventions and make it immediately clear that it returns a boolean.
(defmethod view-mode-supported? :normal [_ _] true)
src/orchard/inspect.clj:1043
- [nitpick] Extract the
add-circlehelper and the joined formatting of supported modes into a separate function (e.g.format-view-mode-line) to declutterrender-view-modeand improve readability.
(defn render-view-mode [inspector]
|
|
||
| (def ^:private view-mode-order [:normal :hex :table :object]) | ||
|
|
||
| (defmulti view-mode-supported? (fn [_inspector view-mode] view-mode)) |
There was a problem hiding this comment.
Add a docstring to view-mode-supported? explaining its parameters (inspector, mode) and return value (boolean) to clarify dispatch behavior.
| (defmulti view-mode-supported? (fn [_inspector view-mode] view-mode)) | |
| (defmulti view-mode-supported? | |
| "Determines whether the given `mode` is supported for the provided `inspector`. | |
| Parameters: | |
| - `inspector`: The current inspector object containing the value and metadata. | |
| - `mode`: The view mode to check for support. | |
| Returns: | |
| - A boolean indicating whether the specified view mode is supported." | |
| (fn [_inspector view-mode] view-mode)) |
Copilot uses AI. Check for mistakes.
Because there are more view modes now, and they all work conditionally, and also to facilitate discovery, I've reworked how they are displayed and changed. Orchard now has its own "toggle view mode" functionality (previously it was in cider-nrepl).
All view modes that are available for the currently inspected objects are displayed, and the enabled one is marked with a special symbol.