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

Edit: Handle conflicting diff decorations #6501

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

umpox
Copy link
Contributor

@umpox umpox commented Jan 2, 2025

Description

Closes https://linear.app/sourcegraph/issue/CODY-4601/disable-autoedits-for-lines-affected-by-edit-command

This PR:

  • Fixes an issue where the diff output of an Edit command would conflict with the diff output of an AutoEdit
  • Fixes an issue where the diff output of an Edit command would conflict with a command hint (e.g. Opt+K to Edit)

We resolve this by assuming that we will not show any other decorations whilst the edit diff output is present.

Test plan

  1. Trigger an Edit command, ensure the diff is visible
  2. Attempt to trigger an AutoEdit (e.g. change some code)
  3. Attempt to trigger a command hint (e.g. highlight some text)

@umpox umpox force-pushed the tr/conflicting-edit-decorations branch from 6da0b9a to d124507 Compare January 2, 2025 15:25
@umpox umpox marked this pull request as ready for review January 2, 2025 15:25
@umpox
Copy link
Contributor Author

umpox commented Jan 2, 2025

@valerybugakov @hitesh-1997 Tagging for early review here.

This works by ensuring we do not show any other decorations when the Edit output is present.

It's fixes the problem, but I think we should consider unifying all of our decoration logic into a central place (e.g. DecorationManager) as they are all implicitly linked to each other in the UX. We could then track what decorations are visible there and handle it more explicitly. That's a bigger bit of work, but something to consider if we find ourselves running into any issues

@umpox umpox force-pushed the tr/conflicting-edit-decorations branch from d124507 to cdb5a4c Compare January 2, 2025 15:28
@valerybugakov
Copy link
Member

I think we should consider unifying all of our decoration logic into a central place

Yes, that sounds great! It will also make it easier to reuse decoration-related logic across different features.

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the quick fix.

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

Successfully merging this pull request may close these issues.

2 participants