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

Add command and button to run tests on current racket file in terminal #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

madkins23
Copy link

Test icon is hand edited copy of run icon so it kind of sucks a little. :-(

Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! 😄

I like the overall idea of adding a command for running tests, but I am bit unsure about adding an icon for it. Most extension only add 1 or 2 icons, so I worry that 3 icons would be too much.

Would you be okay with only having the command without the icon?

@madkins23
Copy link
Author

I'd rather have the icon than the command.

@jryans
Copy link
Collaborator

jryans commented Jul 8, 2023

Hmm, keep in mind that commands can be bound to keyboard shortcuts, so that should make it quite easy to trigger whenever you'd like to use the feature. Hopefully that's at least somewhat better than not having the feature at all...?

We're already exceeding the VS Code UX guidelines which recommend only 1 action with an icon per extension, so I'm fairly hesitant about adding another.

Ideally VS Code itself (or some extension) would make it easy to add your own custom buttons with icons for any command, as I expect different people have different preferences for what should be bound to a displayed button. I can see there is a long-standing feature request for this in VS Code itself.

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