Skip to content
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

cider-eval: support re-rendering the Inspector buffer when cider-inspector-auto-select-buffer is set to nil and there's a *cider-inspect* buffer shown in a non-visible frame #3634

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vemv
Copy link
Member

@vemv vemv commented Mar 14, 2024

My overall intent is the possibility to have the *cider-inspect* buffer permanently on a background frame. cider-eval would have that frame's cider-inspect buffer re-rendered, without switching to it, given that I personally find that only a fraction of evals deserve inspection.

This wasn't possible before, as CIDER would detect that my frame wasn't visible, so it would render the buffer in a random window of the visible frame.

Commits:

  • cider-popup-buffer-display: Do nothing if the intended buffer is visible in another frame and select is nil
    • This seems the right thing to do, for a wide variety of use cases.
  • cider-eval: support re-rendering the Inspector buffer when cider-inspector-auto-select-buffer is set to nil and there's a *cider-inspect* buffer shown in a non-visible frame
    • This supports the workflow of using the inspector occasionally (less intrusively), but ready to be shown anytime.
  • cider-overlays: Make Result truncated message more accurate
    • If there's already a *cider-inspect* buffer visible, we shouldn't suggest the user to inspect the value again.

Cheers - V

…sible in another frame and `select` is nil

This seems the right thing to do.
@vemv vemv requested a review from bbatsov March 14, 2024 07:52
@vemv vemv marked this pull request as draft March 14, 2024 11:11
vemv added 2 commits March 14, 2024 12:25
…nspector-auto-select-buffer` is set to nil and there's a `*cider-inspect*` buffer shown in a non-visible frame

This supports the workflow of using the inspector occasionally (less intrusively), but ready to be shown anytime.
If there's already a `*cider-inspect*` buffer visible, we shouldn't suggest the user to inspect the value again.
@vemv vemv force-pushed the cider-inspect-defcustoms branch from 66b577d to 657b2d3 Compare March 14, 2024 11:28
@vemv vemv changed the title cider-eval: Introduce new cider-auto-inspect-after-eval-select-window defcustom cider-eval: support re-rendering the Inspector buffer when cider-inspector-auto-select-buffer is set to nil and there's a *cider-inspect* buffer shown in a non-visible frame Mar 14, 2024
@vemv vemv marked this pull request as ready for review March 14, 2024 11:36
@vemv
Copy link
Member Author

vemv commented Mar 14, 2024

I reworked this PR to not introduce any defcustoms - ready now!

@bbatsov
Copy link
Member

bbatsov commented Mar 14, 2024

Thanks for the update, I'll review it tomorrow.

@@ -826,6 +826,24 @@ KIND can be the symbols `ns', `var', `emph', `fn', or a face name."
(t x)))
menu-list))

;; Defined here to avoid circular dependencies
(defconst cider-inspector-buffer "*cider-inspect*")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure I like this, because the way I see it the inspector shouldn't be depending on cider-eval and now we're kind of sweeping this odd dependency under the rug.

@bbatsov
Copy link
Member

bbatsov commented Apr 21, 2024

I checked the cider-inspector.el code and it seems the dependency on cider-eval.el is there just to access a single variable from there. Perhaps a better fix would be a simple declare or removing that prefix. After all we're not really doing an eval here anyways.

@bbatsov
Copy link
Member

bbatsov commented Oct 19, 2024

@alexander-yakushev Any thoughts on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants