Evaluate as command #2684
Evaluate as command #2684
Conversation
✅ Deploy Preview for calva-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
CI fails when I add the new file webview.ts, so to work around that I'll just add the webview to snippets
|
So sweet! Thanks for working on this! 🙏 |
There was a problem hiding this comment.
There is a commands reference built in to VS Code. Perhaps it is better to refer to that, than to maintain our own? I see that there's automation to this one, but still. This one is indexed by search engines, so it may be worth it from that standpoint. WDYT?
There was a problem hiding this comment.
I think adding the command reference documentation provides a small amount of value:
- Groups commands into logical topics
- Provides another path for discovering what commands are available
- Indexable by search engines
For users like myself who are discovering functionality, I think it is slightly more inviting to have the grouped topics instead of having to read them all in a seemingly random order.
Because it is generated, the maintenance cost is low.
The motivation I had for adding it was that the existing documentation asked for someone to add it 😄
If we don't want it then I think we should update the documentation to remove the previous confusing message.
| "category": "Calva", | ||
| "command": "calva.info", | ||
| "title": "Show Information Message" | ||
| }, | ||
| { | ||
| "category": "Calva", | ||
| "command": "calva.warn", | ||
| "title": "Show Warning Message" | ||
| }, | ||
| { | ||
| "category": "Calva", | ||
| "command": "calva.error", | ||
| "title": "Show Error Message" | ||
| }, |
There was a problem hiding this comment.
Maybe people can use Joyride to reach functionality like this instead?
There was a problem hiding this comment.
You are correct that notification commands could be left out.
My opinion is that we want to enable toolmakers (like Clay) to do useful things by providing snippets that have "resultAsCommand". The question then is what is useful? Creating a Webview is essential for Clay.
Showing notifications is not essential for Clay. It may be useful for showing info messages or errors, but it is not necessary because those can be done via the Webview.
I think notifications would be broadly useful for toolmakers working on things like standard formatters, test runners, schema validators, and debuggers.
I was surprised that there is no notification command available in VSCode.
VSCode has over 3k commands, Calva has about 100 commands, but none of them handle notification AFAIK. This makes sense because why would a user want to notify themselves? But the intention of this change is to enable toolmakers to request a command, and one obvious thing that toolmakers would benefit from is showing a notification.
This brings up the question of should the feature be "resultAsCommand" at all?
A more conservative approach would be to only provide an interface to the functionality that toolmakers need.
Possibly that doesn't include any commands at all.
I believe that embracing the "command" approach feels nicer, more open, and more powerful for creative customization. And that notifications would be a useful part of that approach.
If adding notifications seems problematic, then I'd like to better understand why, and whether we should be limiting the feature to only show a Webview instead of allowing any command.
Are you concerned about adding notification commands because they would become a breaking change to remove? Perhaps something more?
What has changed?
Fixes #2683
My Calva PR Checklist
I have:
devbranch. (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 eslintbefore creating your PR, or runnpm run eslint-watchto eslint as you go).Ping @PEZ, @bpringe, @corasaurus-hex, @Cyrik