Truncate line when accepting inline suggestions for Supermaven

[kevmo314]

Configures inline completions to delete the remaining text on the given line. This doesn't affect the github copilot inline completion provider since it seems to only generate suggestions if the cursor is at the end of the line but fixes the usability issues related to Supermaven.

Recording.2024-07-19.173713.mp4

Release Notes:

• Fixed Supermaven is unusable in zed #13039

[cla-bot]

We require contributors to sign our Contributor License Agreement, and we don't have @kevmo314 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'.

[kevmo314]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[kynnyhsap]

@kevmo314 i think you put the wrong video in the description. it doesn't really show anything

[kevmo314]

@kevmo314 i think you put the wrong video in the description. it doesn't really show anything

I've updated the screen recording, not sure what happened there.

[ConradIrwin]

@kevmo314 Thanks for taking this on, this has been driving me crazy!

I don't think we should add a TruncationMode concept, and I think we can fix this better without doing that.

Firstly, it seems to be almost? always the case that the generated suffix already exists on the line. It looks like in find_relevant_completion we already remove the prefix that the user has already typed. I think we should update that code to remove any suffix that already exists. That would avoid this weird completion state:

I think we can do that with something like:

        let mut end_of_line = state.prefix_anchor.to_point(buffer);
        end_of_line.column = buffer.line_len(point.row);

        for char in buffer.reversed_chars_for_range(state.prefix_anchor..buffer.anchor_after(end_of_line))
        {
            if let Some(prefix) =  trimmed_completion.strip_suffix(char) {
                trimmed_completion = prefix;
            } else{
                break;
            }
        }

If Supermaven is regularly generating suggestions where the suffix doesn't match, we can keep a fix similar to yours; but instead of passing back an Option<TruncationMode>, lets return an Option<Range<Anchor>>. For now this will always be from the cursor_position to the end_of_line, but it gives us more flexibility in the future.

If you want to pair on getting this over the line, feel free to book time here: https://calendly.com/conradirwin/pairing.

[kevmo314]

That would avoid this weird completion state:

I don't quite follow how that would work. In your screenshot, the completion text ends with ...newline(&completion_text); but the active buffer is (completion_text);. This wouldn't match based on the code you've written?

We can't really guarantee the suffix matches at any regular cadence although @super-jacob can speak more to that probably. Returning an Option<Range<Anchor>> sounds good though if that works for you.

[kevmo314]

Ah, I suppose it would match completion_text); but I'm not completely sure it's the best idea to rely on the suffix being present, wouldn't that edit end up with ...newline(&completion_text);( if accepted?

[ConradIrwin]

🤦 How did I miss that :D.

Let's go with the range approach for now then.

[tomaswarynyca]

@kevmo314 can you please resolve conflicts?

[kevmo314]

@ConradIrwin I've been stuck on this for a bit, how do I get the end of the line in the active_completion_text function with only access to the cursor_position: Anchor and buffer: &Model<Buffer>? In the current version I can do display_map.next_line_boundary() but going through the buffer code no such equivalent function seems to exist. I thought it might be something like what next_line_boundary() is doing but Buffer::line_len() takes a row: u32 and cursor_position: Anchor doesn't seem to contain that information, only the byte offset in the buffer itself.

[ConradIrwin]

You should be able to do something like this:

        let snapshot = buffer.read(cx).snapshot();
        let mut point = cursor_position.to_point(&snapshot);
        point.column = snapshot.line_len(point.row);
        let range = cursor_position..snapshot.anchor_after(point);

If you want to pair on getting this over the line: https://calendly.com/conradirwin/pairing

[WeetHet]

Ah, I suppose it would match completion_text); but I'm not completely sure it's the best idea to rely on the suffix being present, wouldn't that edit end up with ...newline(&completion_text);( if accepted?

What if instead we do not just a suffix comparison, but a full-on diff and display it instead?

[WeetHet]

Something like this:

cc @ConradIrwin

[kevmo314]

Something like this:

cc @ConradIrwin

This seems like the right direction but orthogonal to this change? In particular, to apply this diff one would need to implement the line truncation/deletion range behavior still.

[WeetHet]

Something like this:
cc @ConradIrwin

This seems like the right direction but orthogonal to this change? In particular, to apply this diff one would need to implement the line truncation/deletion range behavior still.

Can we implement them together? I think it would be best for everyone

[kevmo314]

Something like this:
cc @ConradIrwin

This seems like the right direction but orthogonal to this change? In particular, to apply this diff one would need to implement the line truncation/deletion range behavior still.

Can we implement them together? I think it would be best for everyone

I'd like to figure out this PR first in its current scope because it's blocking the use of Supermaven in Zed all together. The rendering seems worthwhile but doesn't really block the use of Supermaven.

[kevmo314]

@ConradIrwin I got the build working with the anchors approach. Works now :)

Recording.2024-07-19.173713.mp4

[ConradIrwin]

@kevmo314 awesome! I just pushed up some cleanup to reduce the change in the editor crate. Will merge when green.

[rgbkrk]

✅