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.

Sign up for GitHub

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

Jump to bottom

inline_assistant: Tabs are overwritten with space characters when first line in selection has no indentation #14885

Closed
1 task done
arturhoo opened this issue Jul 21, 2024 · 1 comment
Closed
1 task done

inline_assistant: Tabs are overwritten with space characters when first line in selection has no indentation #14885

arturhoo opened this issue Jul 21, 2024 · 1 comment
Labels
ai Improvement related to Assistant, Copilot, or other AI features assistant AI feedback for Assistant (inline or panel) defect [core label]

Comments

@arturhoo
Copy link
Contributor

arturhoo commented Jul 21, 2024 •
edited
Loading

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

Steps to reproduce:

  1. Open a go project that uses tabs, for example https://github.com/samber/lo
  2. Trigger the inline assistant while selecting a function with no indentation in the first line of the selection
  3. Observe that the suggestions replace tabs with spaces
image

Environment

Zed: v0.145.1 (Zed Preview)
OS: macOS 14.5.0
Memory: 32 GiB
Architecture: aarch64
@arturhoo arturhoo added admin read Pending admin review defect [core label] triage Maintainer needs to classify the issue labels Jul 21, 2024
@arturhoo arturhoo mentioned this issue Jul 21, 2024
Merged
@arturhoo arturhoo changed the title inline_assistant - tabs are overwritten with space characters when first line in selection has no indentation inline_assistant: Tabs are overwritten with space characters when first line in selection has no indentation Jul 21, 2024
SomeoneToIgnore pushed a commit that referenced this issue Jul 24, 2024
@arturhoo
inline_assistant: Respect tabs when selection first row is not indent…
5ad3c6b 
…ed (#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'
([#14885](#14885)).

<img width="942" alt="image"
src="https://github.com/user-attachments/assets/f2c5d7e9-e8bc-400b-bd6f-09e4a89d22c1">
@JosephTLyons
Copy link
Contributor

This should be included in Zed v0.146.0-pre.

@JosephTLyons JosephTLyons added ai Improvement related to Assistant, Copilot, or other AI features assistant AI feedback for Assistant (inline or panel) and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Jul 24, 2024
CharlesChen0823 pushed a commit to CharlesChen0823/zed that referenced this issue Jul 29, 2024
@arturhoo @CharlesChen0823
inline_assistant: Respect tabs when selection first row is not indent…
90ed24f 
…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 issue Jul 29, 2024
@arturhoo @kevmo314
inline_assistant: Respect tabs when selection first row is not indent…
ca37fe6 
…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">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ai Improvement related to Assistant, Copilot, or other AI features assistant AI feedback for Assistant (inline or panel) defect [core label]
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@arturhoo @JosephTLyons