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

Allow anyone (not just SCM) to hook into quick diff provider #169012

Open
alexr00 opened this issue Dec 13, 2022 · 15 comments
Open

Allow anyone (not just SCM) to hook into quick diff provider #169012

alexr00 opened this issue Dec 13, 2022 · 15 comments
Assignees
Labels
api api-proposal feature-request Request for new features or functionality scm General SCM compound issues
Milestone

Comments

@alexr00
Copy link
Member

alexr00 commented Dec 13, 2022

Why this would be useful: microsoft/vscode-pull-request-github#392

We'll also need to determine what should happen when there are multiple quick diff providers on the same range.

@alexr00 alexr00 added feature-request Request for new features or functionality scm General SCM compound issues labels Dec 13, 2022
@alexr00 alexr00 added this to the January 2023 milestone Dec 13, 2022
@alexr00 alexr00 self-assigned this Dec 13, 2022
alexr00 added a commit that referenced this issue Jan 4, 2023
Part 1: split up the API.
upcoming in part 2: In editor UX for multiple quick diffs.
Fixes #169012
@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Jan 12, 2023
@alexr00 alexr00 reopened this Jan 12, 2023
@VSCodeTriageBot VSCodeTriageBot removed the unreleased Patch has not yet been released in VS Code Insiders label Jan 12, 2023
@alexr00 alexr00 modified the milestones: January 2023, February 2023 Jan 20, 2023
@alexr00
Copy link
Member Author

alexr00 commented Feb 15, 2023

@laurentlb FYI in case you would also find this useful.

@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Feb 16, 2023
@alexr00 alexr00 reopened this Feb 16, 2023
@VSCodeTriageBot VSCodeTriageBot removed the unreleased Patch has not yet been released in VS Code Insiders label Feb 16, 2023
@alexr00 alexr00 modified the milestones: February 2023, Backlog Feb 16, 2023
@alexr00
Copy link
Member Author

alexr00 commented Feb 16, 2023

Here's what it looks like:

Recording 2023-02-16 at 15 43 06

alexr00 added a commit that referenced this issue Feb 16, 2023
alexr00 added a commit that referenced this issue Feb 16, 2023
c-claeys pushed a commit to c-claeys/vscode that referenced this issue Feb 16, 2023
* Add dropdown to quick diff when multiple providers
Fixes microsoft#169012

* Fix git revert action in quick diff
Fixes microsoft#172432

* Make dropdown drive entire quick diff peek
- "x of y" detail
- action bar

* Handle providers being removed

* Delete unused css

* Address PR feedback
c-claeys pushed a commit to c-claeys/vscode that referenced this issue Feb 16, 2023
@justanotheranonymoususer
Copy link
Contributor

Will it also allow to show gutter markers? If yes, perhaps #167306 can be implemented with an extension

@alexr00
Copy link
Member Author

alexr00 commented Mar 6, 2023

Yes, this shows gutter markers.

@eamodio
Copy link
Contributor

eamodio commented Mar 13, 2023

@alexr00 I've tried out this proposal (thrilled about it BTW) and I've run into an issue where it doesn't seem like the quickdiff is removed from the editor properly when disposed. I am currently turning on the feature by calling registerQuickDiffProvider on-demand (in response to a user action) and want to turn it back off when they toggle it back off, but even when disposing the returned disposable it doesn't clear it from the document.

Also minor, but why is there a label both on the registerQuickDiffProvider call and on the provider itself?

@eamodio
Copy link
Contributor

eamodio commented Mar 14, 2023

Playing with this some more (in today's insiders), it seems dispose works more reliably, but still not 100%.

@alexr00
Copy link
Member Author

alexr00 commented Mar 14, 2023

@eamodio there was a dispose that wasn't happening. It is fixed in insiders, though if you're not seeing it work 100% then I'll need to investigate further.

Also minor, but why is there a label both on the registerQuickDiffProvider call and on the provider itself?

This will likely be deduplicated.

@sergei-dyshel
Copy link

Is provideOriginalResource should be called for existing editors at the moment of registration?

I have the following sequence of events (vscode 1.76):

  1. Text editor is showed.
  2. Extension loads and calls registerQuickDiffProvider.
  3. Newly registered quick diff doesn't show.
  4. I close and reopen the editor.
  5. Quick diff is properly displayed in the editor.

@eamodio
Copy link
Contributor

eamodio commented Dec 12, 2023

@alexr00 Any chance this could land soon? I would love to get this in.

@alexr00
Copy link
Member Author

alexr00 commented Dec 12, 2023

@eamodio do you have any other feedback on the API?

Adding to February to revisit then.

@alexr00 alexr00 modified the milestones: Backlog, February 2024 Dec 12, 2023
@eamodio
Copy link
Contributor

eamodio commented Dec 13, 2023

Not really it seems to work well as is. I have a branch with it implemented for the File Changes annotations in GitLens: https://github.com/gitkraken/vscode-gitlens/tree/feature/gutter-changes

Thanks!

@starball5
Copy link

We'll also need to determine what should happen when there are multiple quick diff providers on the same range.

Related? #196759

@alexr00
Copy link
Member Author

alexr00 commented Jan 25, 2024

Moving back to the backlog as we're not going to get to #196759 this month.

@gjsjohnmurray
Copy link
Contributor

@alexr00 are there any plans to finalize this API?

@alexr00
Copy link
Member Author

alexr00 commented Sep 11, 2024

We don't have a plan to finalize it. Tentatively assigning to October to see if we have time then.

@alexr00 alexr00 modified the milestones: Backlog, October 2024 Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api api-proposal feature-request Request for new features or functionality scm General SCM compound issues
Projects
None yet
Development

No branches or pull requests

7 participants