-
Notifications
You must be signed in to change notification settings - Fork 209
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
Edit: Add codelens shortcuts #2757
Conversation
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.
Left comments/questions inline. I'm wondering why the current e2e test is passing when the code lens title has changed 🤔 Do we also want a shortcut for ShowDiff?
lens.command = { | ||
title: 'Retry', | ||
title: `Edit & Retry [${shortcut}]`, |
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.
Do you know why this is not affecting the e2e test where we only look for Edit
?
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.
Where we look for Retry
? I think it's because the text is still there. I'll update the e2e test so it matches exactly
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.
Looking great!
A few things:
- When an edit is running, the cancel shortcut (opt-z) doesn't seem to work
- Can we move Show Diff to the end?
- Can we hide the generic Cody Code Lenses if we have edit code lenses, so we don't get the double up seen in the below?
- I tested out a few different labelling formats (see below), I think fix typo #2 is the best (parentheses) — wdyt?
1:
Without the generic Cody lens:
2:
Without generic Cody lens:
3:
Co-authored-by: Beatrix <68532117+abeatrix@users.noreply.github.com>
Co-authored-by: Beatrix <68532117+abeatrix@users.noreply.github.com>
Co-authored-by: Beatrix <68532117+abeatrix@users.noreply.github.com>
Co-authored-by: Beatrix <68532117+abeatrix@users.noreply.github.com>
Fixed
Done. Do you think we should include a shortcut for it too?
Agree. Updated |
@toolmantim I looked briefly into this. I think we can do this but it'll be a follow up PR and require some refactoring of the Cody codelens so we can easily hide this for a location within a particular document. I think we should also consider what to do about this codelens first. It's disabled by default so we should either enable it fully or rethink how we show it so we can enable it by default |
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.
Looking good!
Done. Do you think we should include a shortcut for [Show Diff] too?
Maybe not, to keep keyboard shortcut footprint low? Or do you think it's weird not having one? Either way, can be a follow-up.
Description
This PR:
Test plan
Single edit:
Multiple edit (actions nearest edit):
Shortcut enablement: