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

Completions: Fix issue where completion sometimes starts in a new line #53246

Merged
merged 2 commits into from
Jun 12, 2023

Conversation

philipp-spiess
Copy link
Contributor

I noticed a small issue when completions are triggered for non-empty lines.

We have one of the three requests that injects a \n at the end of the prefix. This was added back after the trimming logic which of course means that it would include the \n in the result, not something that we want.

We have to evaluate and see if this logic really makes sense. For now it's just a way to "trigger" a response.

Test plan

  • Tested this locally

@philipp-spiess philipp-spiess requested a review from a team June 9, 2023 14:44
@philipp-spiess philipp-spiess self-assigned this Jun 9, 2023
@cla-bot cla-bot bot added the cla-signed label Jun 9, 2023
@github-actions github-actions bot added the team/code-exploration Issues owned by the Code Exploration team label Jun 9, 2023
@philipp-spiess
Copy link
Contributor Author

This one is actually quite noticeable since we prefer responses with more lines of code now, so let's get this out quickly.

@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Jun 9, 2023

📖 Storybook live preview

@philipp-spiess philipp-spiess merged commit 2d1afa9 into main Jun 12, 2023
22 of 23 checks passed
@philipp-spiess philipp-spiess deleted the ps/fix-newlines branch June 12, 2023 10:03
ErikaRS pushed a commit that referenced this pull request Jun 22, 2023
#53246)

I noticed a small issue when completions are triggered for non-empty
lines.

We have one of the three requests that injects a `\n` at the end of the
prefix. This was added back _after_ the trimming logic which of course
means that it would include the `\n` in the result, not something that
we want.

We have to evaluate and see if this logic really makes sense. For now
it's just a way to "trigger" a response.

## Test plan

- Tested this locally

<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
sqs pushed a commit to sourcegraph/cody that referenced this pull request Jul 10, 2023
sourcegraph/sourcegraph#53246)

I noticed a small issue when completions are triggered for non-empty
lines.

We have one of the three requests that injects a `\n` at the end of the
prefix. This was added back _after_ the trimming logic which of course
means that it would include the `\n` in the result, not something that
we want.

We have to evaluate and see if this logic really makes sense. For now
it's just a way to "trigger" a response.

## Test plan

- Tested this locally

<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed team/code-exploration Issues owned by the Code Exploration team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants