Skip to content

Reload content of output view when VS Code is reloaded #2868

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

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from

Conversation

bpringe
Copy link
Member

@bpringe bpringe commented Jun 22, 2025

What has changed?

  • The contents of the output view are reloaded when VS Code is reloaded. (Note: this is different than closing and reopening the output view without closing / reloading VS Code. Contents are not yet preserved in that case.)
  • The scroll position is preserved between VS Code reloads
  • The getState and setState API is used, which will also provides a foundation for persisting contents between closing and reopening the output view without reloading VS Code. This work will be done in another PR.

Fixes #2827

Todo

  • Changed theme should be remembered between closing and reopening
  • Scroll to previous scroll position after VS Code is reloaded
    • Save scrollLeft and scrollHeight on scroll event - maybe throttle
  • Make sure copy button is working after a reload of the window + output view - I saw it not changing to "copied" when clicked one time
    • We need to call highlightElement or the function from highlightjs that highlights all elements after the reload of the output view. The highlightjs-copy plugin provides an "after:highlightElement" function that highlightjs will call after highlighting an element.
    • It seems we need to remove the copy buttons before re-highlighting. Just re-highlighting is not working. This worked when I manually deleted the div with the class hljs-copy-container and then ran highlightAll.
  • Look into error seen in console after reloading VS Code and the output view reloading
    • Uncaught TypeError: Cannot read properties of undefined (reading '__vscode_post_message__')
    • Not sure if this is something we're doing wrong. I don't see anything obvious and things work as they should. I'll leave it for now.
  • Make sure existing tests are passing
  • Add new tests

My Calva PR Checklist

I have:

  • Read How to Contribute.
  • Directed this pull request at the dev branch. (Or have specific reasons to target some other branch.)
  • Made sure I have changed the PR base branch, so that it is not published. (Sorry for the nagging.)
  • Made sure there is an issue registered with a clear problem statement that this PR addresses, (created the issue if it was not present).
    • Updated the [Unreleased] entry in CHANGELOG.md, linking the issue(s) that the PR is addressing.
  • [ ] Figured if anything about the fix warrants tests on Mac/Linux/Windows/Remote/Whatever, and either tested it there if so, or mentioned it in the PR.
  • [ ] Added to or updated docs in this branch, if appropriate
  • Tests
    • Tested the particular change
    • Figured if the change might have some side effects and tested those as well.
  • Formatted all JavaScript and TypeScript code that was changed. (use the prettier extension or run npm run prettier-format)
  • Confirmed that there are no linter warnings or errors (use the eslint extension, run npm run eslint before creating your PR, or run npm run eslint-watch to eslint as you go).

Copy link

netlify bot commented Jun 22, 2025

Deploy Preview for calva-docs ready!

Name Link
🔨 Latest commit 9b78757
🔍 Latest deploy log https://app.netlify.com/projects/calva-docs/deploys/68589059aab3d1000888e265
😎 Deploy Preview https://deploy-preview-2868--calva-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@bpringe bpringe marked this pull request as ready for review June 22, 2025 23:26
@bpringe bpringe requested review from Copilot and PEZ June 22, 2025 23:28
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that the output view’s content is correctly reloaded when VS Code is reloaded, preserving the scroll position and reinitializing copy buttons. Key changes include:

  • Registering a webview serializer to persist and restore the output view state.
  • Updating message keys from :content to more specific names (like :code-theme and :output) and renaming the webview panel identifier.
  • Revising tests and configuration files to reflect the new implementation.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/extension.ts Imports and calls the new registerOutputViewWebviewSerializer function.
src/cljs-lib/test/calva/repl/webview/core_test.cljs Updates expected keys in messages and error texts in tests.
src/cljs-lib/src/calva/repl/webview/ui.cljs Adjusts DOM interaction functions, scroll behavior, and theming; adds new helper functions.
src/cljs-lib/src/calva/repl/webview/core.cljs Centralizes webview panel initialization and message dispatch with updated keys.
shadow-cljs.edn, package.json, CHANGELOG.md Reflect registry and configuration changes for the updated output view.
Comments suppressed due to low confidence (2)

src/cljs-lib/src/calva/repl/webview/core.cljs:176

  • The webview panel identifier has been changed from 'calva:repl-output' to 'calva.output-view'. Ensure that this naming change is consistently updated throughout the project and in any related documentation.
                           "calva.output-view"

src/cljs-lib/src/calva/repl/webview/core.cljs:234

  • The message key has been updated from ':content' to ':output'. Please verify that all message handlers and corresponding tests are updated to reflect this change consistently.
                                                           :output message})

(js/scrollTo x y))

(defn restore-copy-buttons
"Re-initializes copy buttons from CopyButtonPlugin (highlightjs-copy - highlight.js plugin) so they they look
Copy link
Preview

Copilot AI Jun 22, 2025

Choose a reason for hiding this comment

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

There is a duplicate word ('they they') in the docstring for restore-copy-buttons. Please remove the duplicated word for clarity.

Suggested change
"Re-initializes copy buttons from CopyButtonPlugin (highlightjs-copy - highlight.js plugin) so they they look
"Re-initializes copy buttons from CopyButtonPlugin (highlightjs-copy - highlight.js plugin) so they look

Copilot uses AI. Check for mistakes.

@@ -61,7 +61,8 @@
"workspaceContains:**/project.clj",
"workspaceContains:**/shadow-cljs.edn",
"workspaceContains:**/deps.edn",
"onDebugResolve:type"
"onDebugResolve:type",
"onWebviewPanel:calva.output-view"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this? Not saying we don't, just wondering why =)

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