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

New line at the end of suggestions #217

Open
mcrampon opened this issue Nov 19, 2024 · 11 comments
Open

New line at the end of suggestions #217

mcrampon opened this issue Nov 19, 2024 · 11 comments

Comments

@mcrampon
Copy link

Okay I'm not 100% sure on that one, but it looks like some suggestions end with a new line, which makes it quite annoying because most of the time, that new line is not wanted (especially in single-line suggestions). eg.

import Foo from './foo.js';
import CopilotComplete from // ask for completion here
import Bar from './Bar.js';

If this is indeed the case, could the potential \n be removed at the end of the last line of the suggestion?

If that's any help, I'm using phantom suggestions. Not sure if this happens with other modes.

@jfcherng
Copy link
Collaborator

I don't think we know random user codes better than copilot unless it's proven that \n is added by us.

@mcrampon
Copy link
Author

mcrampon commented Nov 19, 2024

I don't think we know random user codes better than copilot

The thing is, the official integration of Copilot in VS Code seems to remove this trailing \n, at least I've never had this issue after 2+ years of using Copilot in VS Code.

unless it's proven that \n is added by us.

I have no idea. Could take a look, but I've never worked on any ST extension before

@jfcherng
Copy link
Collaborator

jfcherng commented Nov 19, 2024

I can't reproduce it in both default popup mode and phantom mode.

ddd_x264.mp4

@mcrampon
Copy link
Author

mcrampon commented Nov 19, 2024

It doesn't seem to happen every time. Had to try a dozen completions before I had it happen in this dummy vue component, for instance:

Untitled.video.-.Made.with.Clipchamp.1.mp4

@jfcherng
Copy link
Collaborator

jfcherng commented Nov 19, 2024

@TerminalFi @timfjord Any thought?

@TerminalFi
Copy link
Owner

@mcrampon I will attempt to reproduce this tomorrow. Until then, if you can provide the logs of when this happens, that would expedite issue identification

@jfcherng
Copy link
Collaborator

Since it's generated by AI, I wouldn't be surprised if the output is random (so may not be reproduced stably).

@mcrampon
Copy link
Author

mcrampon commented Nov 20, 2024

@mcrampon I will attempt to reproduce this tomorrow. Until then, if you can provide the logs of when this happens, that would expedite issue identification

Completely random, I sometimes have a \n, sometimes not:

:: [09:43:49.477] <<< LSP-copilot (17) (duration: 1224ms): {'completions': [{'uuid': 'f629613c-9129-41b7-bb40-9cc670fd168f', 'text': '</script>\n', 'range': {'start': {'line': 7, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'displayText': '</script>\n', 'position': {'line': 7, 'character': 0}, 'docVersion': 10, 'point': 66, 'region': (66, 66)}]}
:: [09:44:39.469] <<< LSP-copilot (21) (duration: 126ms): {'completions': [{'uuid': '794de52d-3c17-44b6-93c0-40714cd6a6eb', 'text': '</script>', 'range': {'start': {'line': 7, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'displayText': '</script>', 'position': {'line': 7, 'character': 0}, 'docVersion': 12, 'point': 66, 'region': (66, 66)}]}
Complete logs if that helps
:: [09:43:48.241]  -> LSP-copilot textDocument/didChange: {'textDocument': {'uri': 'file:///home/mat/foo.vue', 'version': 10}, 'contentChanges': [{'range': {'start': {'line': 6, 'character': 1}, 'end': {'line': 6, 'character': 1}}, 'rangeLength': 0, 'text': '\n'}]}
:: [09:43:48.250] --> LSP-copilot getCompletions (16): {'doc': {'source': "\n\n\n<script>\nexport default {\n  name: 'foo'\n}\n", 'tabSize': 2, 'indentSize': 1, 'insertSpaces': True, 'path': '/home/mat/foo.vue', 'uri': 'file:///home/mat/foo.vue', 'relativePath': '../../foo.vue', 'languageId': 'vue', 'position': {'line': 7, 'character': 0}, 'version': 10}}
:: [09:43:48.252] --> LSP-copilot getCompletionsCycling (17): {'doc': {'source': "\n\n\n<script>\nexport default {\n  name: 'foo'\n}\n", 'tabSize': 2, 'indentSize': 1, 'insertSpaces': True, 'path': '/home/mat/foo.vue', 'uri': 'file:///home/mat/foo.vue', 'relativePath': '../../foo.vue', 'languageId': 'vue', 'position': {'line': 7, 'character': 0}, 'version': 10}}
:: [09:43:48.253] <-  LSP-copilot statusNotification: {'status': 'InProgress', 'message': ''}
:: [09:43:48.255] <-  LSP-copilot statusNotification: {'status': 'InProgress', 'message': ''}
:: [09:43:48.328] <<< LSP-copilot (16) (duration: 78ms): {'completions': [], 'cancellationReason': 'RequestCancelled'}
:: [09:43:49.477] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[fetchCompletions] request.response: [https://proxy.individual.githubcopilot.com/v1/engines/copilot-codex/completions] took 1218 ms'}
:: [09:43:49.477] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] solution 0 returned. finish reason: [stop]'}
:: [09:43:49.477] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] solution 1 returned. finish reason: [stop]'}
:: [09:43:49.477] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] request done: headerRequestId: [f72251e2-7e0e-4042-8172-0fb95a035550] model deployment ID: [d130-20240927144824]'}
:: [09:43:49.477] <-  LSP-copilot statusNotification: {'status': 'Normal', 'message': ''}
:: [09:43:49.477] <<< LSP-copilot (17) (duration: 1224ms): {'completions': [{'uuid': 'f629613c-9129-41b7-bb40-9cc670fd168f', 'text': '</script>\n', 'range': {'start': {'line': 7, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'displayText': '</script>\n', 'position': {'line': 7, 'character': 0}, 'docVersion': 10, 'point': 66, 'region': (66, 66)}]}
:: [09:43:52.852]  -> LSP-copilot textDocument/didChange: {'textDocument': {'uri': 'file:///home/mat/foo.vue', 'version': 11}, 'contentChanges': [{'range': {'start': {'line': 7, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'rangeLength': 0, 'text': '</script>\n'}]}
:: [09:43:52.862] --> LSP-copilot getCompletions (18): {'doc': {'source': "\n\n\n<script>\nexport default {\n  name: 'foo'\n}\n</script>\n", 'tabSize': 2, 'indentSize': 1, 'insertSpaces': True, 'path': '/home/mat/foo.vue', 'uri': 'file:///home/mat/foo.vue', 'relativePath': '../../foo.vue', 'languageId': 'vue', 'position': {'line': 8, 'character': 0}, 'version': 11}}
:: [09:43:52.865] --> LSP-copilot getCompletionsCycling (19): {'doc': {'source': "\n\n\n<script>\nexport default {\n  name: 'foo'\n}\n</script>\n", 'tabSize': 2, 'indentSize': 1, 'insertSpaces': True, 'path': '/home/mat/foo.vue', 'uri': 'file:///home/mat/foo.vue', 'relativePath': '../../foo.vue', 'languageId': 'vue', 'position': {'line': 8, 'character': 0}, 'version': 11}}
:: [09:43:52.865] <-  LSP-copilot statusNotification: {'status': 'InProgress', 'message': ''}
:: [09:43:52.867] <-  LSP-copilot statusNotification: {'status': 'InProgress', 'message': ''}
:: [09:43:52.941] <<< LSP-copilot (18) (duration: 78ms): {'completions': [], 'cancellationReason': 'RequestCancelled'}
:: [09:43:52.981] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[fetchCompletions] request.response: [https://proxy.individual.githubcopilot.com/v1/engines/copilot-codex/completions] took 113 ms'}
:: [09:43:52.984] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] solution 1 returned. finish reason: [stop]'}
:: [09:43:52.984] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] solution 0 returned. finish reason: [stop]'}
:: [09:43:52.984] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] request done: headerRequestId: [eeeeaf8d-a81d-432a-a3b2-abb4b44b0a18] model deployment ID: [d130-20240927144824]'}
:: [09:43:52.984] <-  LSP-copilot statusNotification: {'status': 'Normal', 'message': ''}
:: [09:43:52.984] <<< LSP-copilot (19) (duration: 119ms): {'completions': [{'uuid': 'd3283f61-32cf-4986-893c-bd000d3a5a80', 'text': '```\n', 'range': {'start': {'line': 8, 'character': 0}, 'end': {'line': 8, 'character': 0}}, 'displayText': '```\n', 'position': {'line': 8, 'character': 0}, 'docVersion': 11, 'point': 76, 'region': (76, 76)}]}
:: [09:44:39.329]  -> LSP-copilot textDocument/didChange: {'textDocument': {'uri': 'file:///home/mat/foo.vue', 'version': 12}, 'contentChanges': [{'range': {'start': {'line': 7, 'character': 0}, 'end': {'line': 8, 'character': 0}}, 'rangeLength': 10, 'text': ''}]}
:: [09:44:39.339] --> LSP-copilot getCompletions (20): {'doc': {'source': "\n\n\n<script>\nexport default {\n  name: 'foo'\n}\n", 'tabSize': 2, 'indentSize': 1, 'insertSpaces': True, 'path': '/home/mat/foo.vue', 'uri': 'file:///home/mat/foo.vue', 'relativePath': '../../foo.vue', 'languageId': 'vue', 'position': {'line': 7, 'character': 0}, 'version': 12}}
:: [09:44:39.342] --> LSP-copilot getCompletionsCycling (21): {'doc': {'source': "\n\n\n<script>\nexport default {\n  name: 'foo'\n}\n", 'tabSize': 2, 'indentSize': 1, 'insertSpaces': True, 'path': '/home/mat/foo.vue', 'uri': 'file:///home/mat/foo.vue', 'relativePath': '../../foo.vue', 'languageId': 'vue', 'position': {'line': 7, 'character': 0}, 'version': 12}}
:: [09:44:39.344] <<< LSP-copilot (20) (duration: 4ms): {'completions': [{'uuid': '345b5614-90a2-48f8-b377-bea1b71c8a60', 'text': '</script>', 'range': {'start': {'line': 7, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'displayText': '</script>', 'position': {'line': 7, 'character': 0}, 'docVersion': 12}]}
:: [09:44:39.345] <-  LSP-copilot statusNotification: {'status': 'InProgress', 'message': ''}
:: [09:44:39.462] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[fetchCompletions] request.response: [https://proxy.individual.githubcopilot.com/v1/engines/copilot-codex/completions] took 115 ms'}
:: [09:44:39.469] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] solution 1 returned. finish reason: [stop]'}
:: [09:44:39.469] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] solution 0 returned. finish reason: [stop]'}
:: [09:44:39.469] <-  LSP-copilot window/logMessage: {'type': 3, 'message': '[streamChoices] request done: headerRequestId: [fa2e76ef-81b1-4d1a-a4e7-d2d45ddb487e] model deployment ID: [d130-20240927144824]'}
:: [09:44:39.469] <-  LSP-copilot statusNotification: {'status': 'Normal', 'message': ''}
:: [09:44:39.469] <<< LSP-copilot (21) (duration: 126ms): {'completions': [{'uuid': '794de52d-3c17-44b6-93c0-40714cd6a6eb', 'text': '</script>', 'range': {'start': {'line': 7, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'displayText': '</script>', 'position': {'line': 7, 'character': 0}, 'docVersion': 12, 'point': 66, 'region': (66, 66)}]}
:: [09:44:42.893]  -> LSP-copilot textDocument/didChange: {'textDocument': {'uri': 'file:///home/mat/foo.vue', 'version': 13}, 'contentChanges': [{'range': {'start': {'line': 7, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'rangeLength': 0, 'text': '</script>'}]}
:: [09:44:42.903] --> LSP-copilot getCompletions (22): {'doc': {'source': "\n\n\n<script>\nexport default {\n  name: 'foo'\n}\n</script>", 'tabSize': 2, 'indentSize': 1, 'insertSpaces': True, 'path': '/home/mat/foo.vue', 'uri': 'file:///home/mat/foo.vue', 'relativePath': '../../foo.vue', 'languageId': 'vue', 'position': {'line': 7, 'character': 9}, 'version': 13}}
:: [09:44:42.907] --> LSP-copilot getCompletionsCycling (23): {'doc': {'source': "\n\n\n<script>\nexport default {\n  name: 'foo'\n}\n</script>", 'tabSize': 2, 'indentSize': 1, 'insertSpaces': True, 'path': '/home/mat/foo.vue', 'uri': 'file:///home/mat/foo.vue', 'relativePath': '../../foo.vue', 'languageId': 'vue', 'position': {'line': 7, 'character': 9}, 'version': 13}}
:: [09:44:42.907] <<< LSP-copilot (22) (duration: 3ms): {'completions': []}
:: [09:44:42.909] <<< LSP-copilot (23) (duration: 2ms): {'completions': []}

@timfjord
Copy link
Collaborator

This new line is returned by Copilot, I also noticed that and it is a bit annoying.
But we could definitely work around that. The simplest solution could be to strip the response, as I am almost sure that in 99% of cases, we don't care about the trailing new lines.

@TerminalFi
Copy link
Owner

Let me look at the vscode and see the logic

@timfjord
Copy link
Collaborator

I haven't checked, but most likely in VSCode they rely on the inline suggestions API, which can handle this case, because usually, after suggestion, there is a new line, so it might just ignore it.
In our case, since ST doesn't offer something similar, we should probably manually trim the trailing new lines.

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

No branches or pull requests

4 participants