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

SCM diff gutter is not working since v1.48 #104640

Closed
JohnstonCode opened this issue Aug 14, 2020 · 22 comments
Closed

SCM diff gutter is not working since v1.48 #104640

JohnstonCode opened this issue Aug 14, 2020 · 22 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders scm General SCM compound issues verified Verification succeeded
Milestone

Comments

@JohnstonCode
Copy link

JohnstonCode commented Aug 14, 2020

  • VSCode Version: v1.48
  • OS Version: Fedora 32

Steps to Reproduce:

  1. Use the vscode extension
  2. Make changes to a tracked file
  3. No gutter appears

Does this issue occur when all extensions are disabled?: Yes (with all other extensions apart from the svn one)

Works in VSCode v1.47.3

@JohnstonCode
Copy link
Author

@jordankaiser
Copy link

jordankaiser commented Aug 14, 2020

Confirming the same behavior, diff gutters are gone.

VS Code: Version: 1.48.0
OS: Mac OS 10.12.4

@hdpoliveira
Copy link

Confirming the same behavior in vscode-hg: mrcrowl/vscode-hg#158

@hdpoliveira
Copy link

Looking at git extension, seems it is using registerFileSystemProvider, while vscode-hg uses registerTextDocumentContentProvider. I'm not sure what the differences are between the two.

@hdpoliveira
Copy link

I've reimplemented Mercurial's context provider in terms of registerFileSystemProvider and the gutter indicator works.

Used https://github.com/microsoft/vscode/blob/master/extensions/git/src/fileSystemProvider.ts as reference.

@hdpoliveira
Copy link

hdpoliveira commented Aug 15, 2020

@joaomoreno @mjbvz please let us know if SCM extensions should be moved to use FileSystemProvider or stick to TextDocumentContentProvider

FileSystemProvider was added to git on 7 Nov 2019 (5d60b7f)

TextDocumentContentProvider was removed from git on 15 Apr 2020 (b8512b2)

@isidorn
Copy link
Contributor

isidorn commented Aug 17, 2020

since @joaomoreno is on vacation forwarding to @eamodio as he might know more

@eamodio
Copy link
Contributor

eamodio commented Aug 17, 2020

@hdpoliveira As for a recommendation, I would say it depends on the use-case. If you were to start today, I would definitely recommend using a FileSystemProvider (even though its harder to use, it provides the ability to do a lot more and better deals with any encoding issues (since it uses the built-in vscode encoding mechanisms). At the same time, if you have a narrower use-case and TextDocumentContentProvider is fulfilling your needs, I don't see a reason to switch. And we aren't planning to deprecate the TextDocumentContentProvider or anything.

I will dig into what changed with the TextDocumentContentProvider that's causing this issue.

@hdpoliveira
Copy link

Hi @eamodio !
I decided to switch to FileSystemProvider in Mercurial extesion, given that Git extension is using it and Perforce extension is too.

@eamodio
Copy link
Contributor

eamodio commented Aug 18, 2020

I'm pretty sure this was introduced because of this commit: 003a367, but still investigating a fix

@isidorn
Copy link
Contributor

isidorn commented Aug 18, 2020

@eamodio thanks for investigating, should this be a candidate (sounds like a regression, but not high impact). You would know better.

@henry000dev
Copy link

I can also confirm with @JohnstonCode, except my OS is Windows 10. I'm using SVN as the SCM.

@henry000dev
Copy link

The issue has gone away - I can see the gutters are now available again with release 1.48.2.

@RLThomaz
Copy link

The issue has gone away - I can see the gutters are now available again with release 1.48.2.

The issue is still there for me - Ubuntu 18.04, vscode 1.48.2.

@brneor
Copy link

brneor commented Aug 26, 2020

Can confirm gutter diff is showing up again on 1.48.2 under Windows and Linux.

@RLThomaz
Copy link

Okay, gutter diff is showing up again, but only if you get the latest version of SVN-SCM (JohnstonCode/svn-scm#1053), the fix was not due to the new VSCode release (1.48.2).

@joaomoreno joaomoreno added this to the August 2020 milestone Aug 31, 2020
@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug scm General SCM compound issues labels Aug 31, 2020
@joaomoreno
Copy link
Member

@joaomoreno @mjbvz please let us know if SCM extensions should be moved to use FileSystemProvider or stick to TextDocumentContentProvider

This was definitely an unintended breakage. Looking into where the assumption actually breaks down...

@joaomoreno
Copy link
Member

I believe the wrong assumption lies here:

this._originalModel = ref.object as IResolvedTextFileEditorModel;

This breaks down with model references coming in from TextDocumentContentProvider, in ref.object is an instance of ResourceEditorModel which does not implement IResolvedTextFileEditorModel. @bpasero I remember we did this together, can you guide towards a solution here?

@joaomoreno
Copy link
Member

Thanks to @bpasero for the help in fixing this!

@miguelsolorio miguelsolorio added the verified Verification succeeded label Sep 3, 2020
@miguelsolorio
Copy link
Contributor

Confirmed that this works now works using an svn project:

image

@DavidGoldman
Copy link
Contributor

DavidGoldman commented Sep 3, 2020

When will this fix be released? v1.49?

@miguelsolorio
Copy link
Contributor

miguelsolorio commented Sep 3, 2020

@DavidGoldman yes, this will be in out in the August release (1.49) which should drop next week. It's already fixed on our Insiders (nightly build). You can also use Settings Sync to sync your settings/extensions/keybindings between Insiders and Stable.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders scm General SCM compound issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

12 participants