-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
Add basic REPL History search #2540
base: dev
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for calva-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
function searchReplHistory(): void { | ||
const editor = util.getActiveTextEditor(); | ||
const history = getHistory(getSessionType()); | ||
vscode.window.showInputBox({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be this dangling promise that causes the linter to complain. I think the solution is to return the promise, because otherwise systems like Joyride and runCommands
can't wait for the command to finish, which limits its usefulness.
vscode.window.showInputBox({ | |
return vscode.window.showInputBox({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Thanks for helping improve Calva! 🙏
Before we will consider merge, you'll need to reinstall the checklist of the PR and tick items off as you have completed them. Don't hesitate to ask us for help if there's anything that is unclear or you just want help with it for whatever reason.
This implementation is very basic. It is missing a few things:
I'm curious how you would've implemented these. ChatGPT suggests to use a custom view. Maybe that's what I'll try next. But so far I've compiled the version of the plugin to test with and want to see how much those issues would affect me. |
It's important that this is kept simple and low maintenance. I don't think anyone in the Calva team really uses the REPL Window much and we're not keen on building it out much more. I think that eventually we may replace it by something simpler alltogether. I think I would try with a quickpick menu first. Building it from the full repl history. That would offload the search and match to VS Code. Joyride example: (vscode/window.showQuickPick
#js [#js {:label "(when-let [requires (resolve 'clojure.main/repl-requires)]\n(clojure.core/apply clojure.core/require @requires))"
:detail "[clj] user"}
#js {:label "(println 42)"
:detail "[cljs] pez.pirate-lang"}]) This pops open a quickpick menu with the two items, showing the start of both. When the user types, any matching text will be highlighted in the menu and the menu will shrink to only the matchin items. It has limitations. Only the start of the text is shown, and if the match is outside the visible text, it will be invisible. But the menu will still shrink to only the matching items, so I think it is good enough and worth to try. |
Interesting. Do you refer to all Calva users or just some core members? |
I'm referring to the maintainers of Calva. |
What has changed?
This implements a very basic REPL History search.
Fixes #2175
My Calva PR Checklist
I have:
dev
branch. (Or have specific reasons to target some other branch.)published
. (Sorry for the nagging.)[Unreleased]
entry inCHANGELOG.md
, linking the issue(s) that the PR is addressing.npm run prettier-format
)npm run eslint
before creating your PR, or runnpm run eslint-watch
to eslint as you go).Ping @PEZ, @bpringe, @corasaurus-hex, @Cyrik