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

update: remove description requirement from custom command #3025

Merged
merged 6 commits into from
Feb 7, 2024

Conversation

abeatrix
Copy link
Contributor

@abeatrix abeatrix commented Feb 2, 2024

Remove the requirement to add a description for a custom command as the prompt of the command is descriptive enough.

This eliminates one unnecessary step to simplify the custom command creation process.

Based on screenshot shared by our users, it looks like users often use the name of the command to fill in the description field, e.g. https://sourcegraph.slack.com/archives/C04MSD3DP5L/p1706661347779839?thread_ts=1706656724.966049&cid=C04MSD3DP5L, https://discord.com/channels/969688426372825169/1202562648340701257/1202562648340701257, https://sourcegraph.slack.com/archives/C05MW2TMYAV/p1703011617179849

Feedback from a power user:

One piece of feedback when defining your custom commands is to get rid of the command description step.

Test plan

We will still show the description in the command menu when provided, but it is no longer a required field when creating a command from the command palette. You can try to create a custom command with and without the "description" field to confirm.

I've updated the current e2e tests and unit tests for custom commands to reflect this change.

@abeatrix abeatrix requested review from toolmantim and a team February 7, 2024 00:39
Comment on lines 226 to 232
// Show confirmation for user setting only, as
// workspace config can be reverted with git history.
vscode.window
.showInformationMessage(
'Remove the Cody Custom Command file from your User setting?',
'Confirm'
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just show the confirmation consistently for both, as even if it's in Git history I think deletion deserves a confirmation.

For the confirmation, it's always good to use the action itself as button label, rather than the generic "Confirm" text, i.e. "Remove Command"

For the message can we make it like the following (matching the path labels we use in the Configure Custom Commands quickpick too):

  • "Remove custom command 'My Thing' from User Settings (~/.vscode/cody.json)?"
  • "Remove custom command 'My Thing' from Workspace Settings (.vscode/cody.json)?"

CleanShot 2024-02-07 at 15 26 17@2x

So in the end something like:

vscode.window
    .showInformationMessage(
        'Remove custom command \'My Thing\' from User Settings (~/.vscode/cody.json)?',
        'Remove Command'
    )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@toolmantim Oh this is not removing a single command, but the whole file. So something like this?

vscode.window
    .showInformationMessage(
        'Remove user settings file (~/.vscode/cody.json)?',
        'Remove File'
    )

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right! I forgot.

In that case let's call it Delete (same as right clicking on a file in the VS Code file explorer tree).

They then show this:

We should probably add { useTrash: true } to the vscode.workspace.fs.delete call too, and do something similar:

vscode.window
    .showInformationMessage(
        'Are you sure you want to delete your Cody user settings file \'~/.vscode/cody.json\'?',
        { detail: 'You can restore this file from the Trash.' }
        'Move to Trash'
    )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use modal:

image

@abeatrix
Copy link
Contributor Author

abeatrix commented Feb 7, 2024

Will update the e2e test tmr

@abeatrix abeatrix merged commit a1afc07 into main Feb 7, 2024
15 checks passed
@abeatrix abeatrix deleted the bee/update-cc branch February 7, 2024 21:22
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.

None yet

2 participants