inline_assistant: Respect tabs when selection first row is not indented #14886
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
We require contributors to sign our Contributor License Agreement, and we don't have @arturhoo on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check |
cla-bot
bot
added
the
cla-signed
|
The cla-bot has been summoned, and re-checked this pull request! |
maxdeviant
changed the title
arturhoo
force-pushed
the
inline_assistant/respect-tabs
branch
3 times, most recently
from
9d4c82d
to
9807631
Compare
arturhoo
force-pushed
the
inline_assistant/respect-tabs
branch
from
9807631
to
84f3d19
Compare
SomeoneToIgnore
approved these changes
Jul 24, 2024
SomeoneToIgnore
added a commit
that referenced
this pull request
Jul 24, 2024
Follow-up of #14886 We do not require the branch to be up-to-date with `main` before merging, and in 4 days some related test code got reworked so that there were no conflicts and it slipped. Release Notes: - N/A
CharlesChen0823
pushed a commit
to CharlesChen0823/zed
that referenced
this pull request
Jul 29, 2024
…ed (zed-industries#14886) When using the inline assistant with a language such as Go that uses tabs, if the user selects a block of text that is correctly formatted and where the first line has no indentation, the `suggested_line_indent` variable ends up with `IndentSize { len: 0, kind: Space }`. That's because `suggested_line_indent` current relies on `BufferSnapshot::suggested_indents` suggestion for the first line on the selection, but since it is already correctly indented, there are no suggestions and `MultiBufferSnapshot::indent_size_for_line` is used instead. https://github.com/zed-industries/zed/blob/2d96bba61fd9e60951ecfcf697707a974475c1b2/crates/assistant/src/inline_assistant.rs#L2124-L2128 In this patch, we also take a look at the rest of the selection and detect tabs. If one is encountered, we assume that tabs should always be used. I suppose this isn't perfect, especially if the original file had a mix of spaces and tabs, however it seems better than the status quo. I considered using `BufferSnapshot::language_indent_size_at`, but I imagine tabs should be preserved even when a specific language isn't being used. See screenshot below of the original prompt with this patch. Tests: * New unit test * I've also manually tested with a few other cases: selection where all lines are indented and file that only use spaces. Release Notes: - Fixed 'inline_assistant: tabs are overwritten with space characters when first line in selection has no indentation' ([zed-industries#14885](zed-industries#14885)). <img width="942" alt="image" src="https://github.com/user-attachments/assets/f2c5d7e9-e8bc-400b-bd6f-09e4a89d22c1">
CharlesChen0823
pushed a commit
to CharlesChen0823/zed
that referenced
this pull request
Jul 29, 2024
Follow-up of zed-industries#14886 We do not require the branch to be up-to-date with `main` before merging, and in 4 days some related test code got reworked so that there were no conflicts and it slipped. Release Notes: - N/A
kevmo314
pushed a commit
to kevmo314/zed
that referenced
this pull request
Jul 29, 2024
…ed (zed-industries#14886) When using the inline assistant with a language such as Go that uses tabs, if the user selects a block of text that is correctly formatted and where the first line has no indentation, the `suggested_line_indent` variable ends up with `IndentSize { len: 0, kind: Space }`. That's because `suggested_line_indent` current relies on `BufferSnapshot::suggested_indents` suggestion for the first line on the selection, but since it is already correctly indented, there are no suggestions and `MultiBufferSnapshot::indent_size_for_line` is used instead. https://github.com/zed-industries/zed/blob/2d96bba61fd9e60951ecfcf697707a974475c1b2/crates/assistant/src/inline_assistant.rs#L2124-L2128 In this patch, we also take a look at the rest of the selection and detect tabs. If one is encountered, we assume that tabs should always be used. I suppose this isn't perfect, especially if the original file had a mix of spaces and tabs, however it seems better than the status quo. I considered using `BufferSnapshot::language_indent_size_at`, but I imagine tabs should be preserved even when a specific language isn't being used. See screenshot below of the original prompt with this patch. Tests: * New unit test * I've also manually tested with a few other cases: selection where all lines are indented and file that only use spaces. Release Notes: - Fixed 'inline_assistant: tabs are overwritten with space characters when first line in selection has no indentation' ([zed-industries#14885](zed-industries#14885)). <img width="942" alt="image" src="https://github.com/user-attachments/assets/f2c5d7e9-e8bc-400b-bd6f-09e4a89d22c1">
kevmo314
pushed a commit
to kevmo314/zed
that referenced
this pull request
Jul 29, 2024
Follow-up of zed-industries#14886 We do not require the branch to be up-to-date with `main` before merging, and in 4 days some related test code got reworked so that there were no conflicts and it slipped. Release Notes: - N/A
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using the inline assistant with a language such as Go that uses tabs, if the user selects a block of text that is correctly formatted and where the first line has no indentation, the
suggested_line_indentvariable ends up withIndentSize { len: 0, kind: Space }. That's becausesuggested_line_indentcurrent relies onBufferSnapshot::suggested_indentssuggestion for the first line on the selection, but since it is already correctly indented, there are no suggestions andMultiBufferSnapshot::indent_size_for_lineis used instead.zed/crates/assistant/src/inline_assistant.rs
Lines 2124 to 2128 in 2d96bba
In this patch, we also take a look at the rest of the selection and detect tabs. If one is encountered, we assume that tabs should always be used. I suppose this isn't perfect, especially if the original file had a mix of spaces and tabs, however it seems better than the status quo.
I considered using
BufferSnapshot::language_indent_size_at, but I imagine tabs should be preserved even when a specific language isn't being used.See screenshot below of the original prompt with this patch.
Tests:
Release Notes: