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

fix: doc command #1273

Merged
merged 18 commits into from
Oct 3, 2023
Merged

fix: doc command #1273

merged 18 commits into from
Oct 3, 2023

Conversation

abeatrix
Copy link
Contributor

@abeatrix abeatrix commented Oct 2, 2023

I noticed the /doc command built from the current main branch has the following issues:

  • a new empty line is added before the
  • if you run the /doc command and then reload VS code, your chat will start with the output of your last /doc command in chat view

image

This PR fixes the issues listed above.

Also merged the prompt with the one tested by Tim in #1198

IMPORTANT: This PR depends on the prompt improvement made in #1198

Test plan

  • highlight a function or method in the editor
  • right click on the highlighted code, then select Cody>Document Code
  • Expected behavior: Docstring added directly above the highlighted code without an extra new line added at the end

Before

https://www.loom.com/share/c1d316d9031d4550952da81d5e5c61fd?sid=3c06e825-e6c3-4c43-a17b-e93f186947a3

before.pull_1273.mp4

After

https://www.loom.com/share/accef38b7f0f4d0c8e1ba8b89581acd2?sid=29fe90a4-9f5e-4a51-8c40-f4adad1c848f

after.pull_1273.mp4

Copy link
Contributor

@toolmantim toolmantim left a comment

Choose a reason for hiding this comment

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

Tested the "if you run the /doc command and then reload VS code, your chat will start with the output of your last /doc command in chat view" part of this and works as expected 👍🏼 But I reckon let's leave the prompt changes to the other PR.

@abeatrix
Copy link
Contributor Author

abeatrix commented Oct 3, 2023

Tested the "if you run the /doc command and then reload VS code, your chat will start with the output of your last /doc command in chat view" part of this and works as expected 👍🏼 But I reckon let's leave the prompt changes to the other PR.

agree. this PR should not be merged until #1198 is merged

Copy link
Contributor

@dominiccooney dominiccooney left a comment

Choose a reason for hiding this comment

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

Some feedback inline.

I think we should try to simplify this a bit—only strip one newline, not multiple ones; not prune human text from the transcript?

vscode/src/chat/MessageProvider.ts Outdated Show resolved Hide resolved
vscode/src/non-stop/FixupController.ts Outdated Show resolved Hide resolved
vscode/src/non-stop/FixupController.ts Outdated Show resolved Hide resolved
lib/shared/src/chat/transcript/interaction.ts Outdated Show resolved Hide resolved
lib/shared/src/chat/transcript/index.ts Outdated Show resolved Hide resolved
abeatrix and others added 2 commits October 2, 2023 18:54
Co-authored-by: Dominic Cooney <dominic.cooney@sourcegraph.com>
Copy link
Contributor

@dominiccooney dominiccooney left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you!

vscode/src/non-stop/FixupController.ts Outdated Show resolved Hide resolved
abeatrix and others added 2 commits October 2, 2023 21:38
Co-authored-by: Dominic Cooney <dominic.cooney@sourcegraph.com>
@abeatrix
Copy link
Contributor Author

abeatrix commented Oct 3, 2023

@dominiccooney thank you for your thoughtful review as always 🙇‍♀️

@philipp-spiess philipp-spiess merged commit 532ffa5 into main Oct 3, 2023
12 checks passed
@philipp-spiess philipp-spiess deleted the bee/fix-doc-command branch October 3, 2023 11:23
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.

4 participants