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

Edit: Add further code actions for editing and documenting code #1724

Merged
merged 21 commits into from
Nov 23, 2023

Conversation

umpox
Copy link
Contributor

@umpox umpox commented Nov 13, 2023

Description

This PR:

  • Adds 3 new code actions
  • Document code action
  • Edit code action
    • Shown whenever there is a user selection
    • Opens the edit instruction input so the user can type how they want the code to be edited
  • Generate code action
    • Shown whenever there is no selection and the current line is empty
    • Opens the edit instruction input so the user can type what they want to generate

Document Code Action

Only shows when on a documentable symbol. Determined via Tree Sitter

CodeActionDocument.mov

Edit Code Action

Only shows when there is an active selection

CodeActionEdit.mov

Generate Code Action

Only shows when there is no selection and the current line is empty.

CodeActionGenerate.mov

Test plan

Trigger code actions on:

  • Symbols that could be documented
  • Selections in a document
  • Empty lines with no selections

@umpox umpox mentioned this pull request Nov 13, 2023
@umpox umpox self-assigned this Nov 13, 2023
Base automatically changed from tr/better-fix-context to main November 14, 2023 11:21
@umpox umpox force-pushed the tr/add-further-code-actions branch 2 times, most recently from 4ca05bf to 60f800b Compare November 15, 2023 10:16
@umpox umpox changed the base branch from main to vb/move-tree-sitter-out-of-completions November 15, 2023 14:07
@valerybugakov valerybugakov force-pushed the vb/move-tree-sitter-out-of-completions branch from 8772cfa to eb05429 Compare November 16, 2023 02:41
Base automatically changed from vb/move-tree-sitter-out-of-completions to main November 16, 2023 02:44
@umpox umpox force-pushed the tr/add-further-code-actions branch from 567e763 to c20632c Compare November 16, 2023 08:45
@umpox umpox marked this pull request as ready for review November 16, 2023 10:03
Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Got language blocks leaking into the document when using the "generate" action:

Screenshot 2023-11-17 at 12 51 24

Filepath: vscode/src/logged-rerank.ts
Prompt: function that outputs ASCII art to the console with a cat image

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

The "document" action removed the interface field a couple of times after adding a comment. For example, here, I asked it to document the triggerKind property:

Screenshot 2023-11-17 at 12 56 12

Filepath: src/completions/get-inline-completions.ts
Property: InlineCompletionsParams['triggerKind']

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

The "edit" action most worked as expected, but the results show that the LLM has a poor understanding of the surrounding code. The UX is awesome. I'd love to use it with a smarter model and better context. Noob q: what model do we use for quick actions?

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Love the new actions! Especially "document" and "generate" ones. Going to use them locally and report issues in Slack 🙌

public static readonly providedCodeActionKinds = [vscode.CodeActionKind.RefactorRewrite]

public provideCodeActions(document: vscode.TextDocument, range: vscode.Range): vscode.CodeAction[] {
const [documentableNode] = execQueryWrapper(document, range.start, 'getDocumentableNode')
Copy link
Member

Choose a reason for hiding this comment

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

Noice!

Copy link
Member

Choose a reason for hiding this comment

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

@umpox, let me know if you noticed any confusing parts about our tree-sitter setup 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

This is super cool! Can we add it as an option for the commands/custom commands? Think it'd be helpful with the /explain and /test commands!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@valerybugakov The tests are great and the SDK is super easy to use and build from.

I found it helpful to take the contents from a page at https://learnxinyminutes.com/ and then put it into this playground when writing the queries. Covers quite a lot of cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abeatrix Maybe, do you have any ideas? I think we could possibly use this with the scope expansion too, e.g. expanding to nearest function etc if we want more control over different scenarios.

I think we don't make this available for custom commands yet, it's quite hard to write these queries so not sure if anyone will want to integrate them with a custom command. Possibly there's future work to integrate pre-written ones though, like useNearestFunctionAsSelection or something

Copy link
Member

Choose a reason for hiding this comment

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

I found it helpful to take the contents from a page at https://learnxinyminutes.com/

TIL, thanks for sharing! I've been asking GPT-4 to generate a file with all syntax constructs, but using the website you shared is a bit more reliable 😛

Copy link
Contributor

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

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

Found some issues with the code actions, but love the idea in general! Curious to know why we need a different prompt and action for the Documentation Code Action instead of using the existing one?

Document Code Lens

Break the code and the undo button does not work

Screen.Recording.2023-11-17.at.6.24.07.AM.mov

Undo button always remove the last line that it did not line

Issue unrelated to this PR but related to the feature 😅

Screen.Recording.2023-11-17.at.6.21.29.AM.mov

Retry Code Lens breaks the code before applying new changes

Screen.Recording.2023-11-17.at.6.15.04.AM.mov

selection?: vscode.Selection
): vscode.CodeAction {
const action = new vscode.CodeAction(displayText, vscode.CodeActionKind.RefactorRewrite)
const source = 'code-action'
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding the type of action to the end of source, e.g. code-action:generate, code-action:edit so we can create a dashboard for each code action usages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep good idea

@umpox
Copy link
Contributor Author

umpox commented Nov 22, 2023

@abeatrix Thanks for reporting the "undo" bugs! I took a look and everything you mentioned was related to weird "undo" logic we had.

The problem was that we were incorrectly using the selectionRange when appending or prepending new code to the target range. Our code assumed that any edits that happened immediately before or after the target range, was completely unrelated - so it wouldn't include this in the edit, or in the code lens or diff.

For fixups we do care about appending or prepending, so I've updated this logic to support expanding the range if there is a partial overlap (change.range.end === selectionRange.start or change.range.start === selectionRange.end).

This also has the benefit of improving the diff highlighting for the /doc output, as previously we mostly ignored this.

@umpox umpox requested a review from abeatrix November 22, 2023 14:45
@abeatrix
Copy link
Contributor

Thanks for working on this!

Confirmed the following are fixed

  • Document Code Lense
  • Undo button always remove the last line that it did not line
  • Retry Code Lens breaks the code before applying new changes
Screen.Recording.2023-11-22.at.8.31.22.AM.mov

There are some issues with theDocument Code Lensewhere the generated doc string will replace the current line of text, and the undo button will remove the last line of the current range:

Screen.Recording.2023-11-22.at.8.37.37.AM.mov

@umpox
Copy link
Contributor Author

umpox commented Nov 23, 2023

@abeatrix Can't replicate the issue with the line being removed 🤔 Please could you output the logs if you see it again?

The only things I can think are:

  • Maybe the formatter you use has different behavior to mine
  • The LLM had a moment and produced some weird output

Going to update this with main and merge this (aware you're on PTO until next week) to avoid long lived branches. Further work to imrpove the response reliablity is related to this issue #1790

@umpox umpox dismissed abeatrix’s stale review November 23, 2023 14:10

PTO & following up on LLM response reliability in #1790

@umpox umpox enabled auto-merge (squash) November 23, 2023 14:13
@umpox umpox merged commit 4a38ea5 into main Nov 23, 2023
14 checks passed
@umpox umpox deleted the tr/add-further-code-actions branch November 23, 2023 14:31
@umpox umpox assigned umpox and unassigned umpox Nov 30, 2023
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

3 participants