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

Adopt FileSystemProvider for Git #55110

Closed
eamodio opened this issue Jul 26, 2018 · 13 comments · Fixed by #84130
Closed

Adopt FileSystemProvider for Git #55110

eamodio opened this issue Jul 26, 2018 · 13 comments · Fixed by #84130
Assignees
Labels
feature-request Request for new features or functionality git GIT issues on-release-notes Issue/pull request mentioned in release notes on-testplan
Milestone

Comments

@eamodio
Copy link
Contributor

eamodio commented Jul 26, 2018

I'm trying to fix gitkraken/vscode-gitlens#430 and I think my best option it to stop creating temp files and instead leverage the built-in git content provider (git: scheme). I have this solution pretty much working (though will require a bunch of burn in), but the biggest issue I'm having is when I'm showing a diff of a file that has been renamed or moved the open file command will fail because the path of the file doesn't match the working tree.

To fix this can we add an optional workingPath to GitUriParams and use it if it exists when executing the open file command?

The other challenge is that using the git content provider doesn't handle images, but I can probably just replicate the logic here:

https://github.com/Microsoft/vscode/blob/master/extensions/git/src/commands.ts#L255-L272

/cc @joaomoreno

@vscodebot vscodebot bot added the git GIT issues label Jul 26, 2018
@joaomoreno
Copy link
Member

joaomoreno commented Aug 2, 2018

I see. We have the notion of the rename path in our Git model, but not in the URI. How else could a Git extension API help you, instead of having additional fields on git uris?

@joaomoreno joaomoreno added the info-needed Issue requires more information from poster label Aug 2, 2018
@vscodebot vscodebot bot closed this as completed Aug 9, 2018
@vscodebot
Copy link

vscodebot bot commented Aug 9, 2018

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!

@joaomoreno joaomoreno reopened this Aug 9, 2018
@eamodio
Copy link
Contributor Author

eamodio commented Aug 9, 2018

Doh -- got closed. I guess fixing the open file command, to detect the case where the file on disk doesn't exist and walk back from that commit to the working file -- which is what I do in GitLens in this case (and then store that working filename in the Uri)

@joaomoreno
Copy link
Member

@eamodio Let's push for this. Would exposing this getURI call be enough?

@eamodio
Copy link
Contributor Author

eamodio commented Sep 19, 2018

I think that will help some, but I've recently abandoned the usage of a TextContentProvider in favor of implementing a FileSystemProvider for git (see https://github.com/eamodio/vscode-gitlens/blob/develop/src/git/fsProvider.ts). Using that structure, it solves the encoding issues and image issues since it leverages all the existing work in vscode to handle those things. I also added it for more features than just to replace the TextContentProvider but so far it seems to be working out quite well.

2 other things that I think would help would be to somehow provide the Uri (or something) to the Open File command that is added in a diff view -- to handle renames, etc. And the other I would love, is a way to provide a title for an editor (like you can for a diff view), because right now formatting non-file Uri's in a way that is friendly in the editor tab is a pain.

@joaomoreno
Copy link
Member

joaomoreno commented Sep 20, 2018

I think that will help some, but I've recently abandoned the usage of a TextContentProvider in favor of implementing a FileSystemProvider for git

This is probably something that should also be done by git itself. Would possibly fix these issues: #36219, #48038, #48038

Maybe we can join forces, bring that over to the git extension and you could leverage it?

somehow provide the Uri (or something) to the Open File command that is added in a diff view -- to handle renames, etc.

Hm... not sure I understand this one.

And the other I would love, is a way to provide a title for an editor (like you can for a diff view), because right now formatting non-file Uri's in a way that is friendly in the editor tab is a pain.

Just talked to @bpasero about this yesterday. Guess what? 4e3f58f#diff-81002b6983f6ca938fb1efc58cd9da2f

@eamodio
Copy link
Contributor Author

eamodio commented Sep 20, 2018

Yeah, I was talking to @rebornix a bit about that the other day. As it should remove a bunch of the complexities that the git extension currently has to deal with images, encoding, etc. I am definitely open to join forces to get this into the git extension itself. I'm also planning on implementing a search provider (as soon as that API is stable) so that you can search a git repo at a specific revision -- is that something else that could go in core?

somehow provide the Uri (or something) to the Open File command that is added in a diff view -- to handle renames, etc.

I mean that if I open a diff of abc.txt @ revision xyz, but in the working tree abc.txt is now named abc.md -- the Open File command will try to open abc.txt and fail. So for GitLens revisions, I have my own Open File Revision and Open Working File and Open Working File handles the renames and finds the correct working file.

Re: label

Yey! Although this is only helpful when you are explicitly opening a Uri. But in the case of you opening a folder with a FileSystemProvider uri, then there is no way to provide that label. Instead it would have to somehow be encoded or something into the uri itself or something -- I dunno, its a very tricky one.

@joaomoreno
Copy link
Member

I'm also planning on implementing a search provider (as soon as that API is stable) so that you can search a git repo at a specific revision -- is that something else that could go in core?

It is definitely interesting!

I mean that if I open a diff of abc.txt @ revision xyz, but in the working tree abc.txt is now named abc.md -- the Open File command will try to open abc.txt and fail. So for GitLens revisions, I have my own Open File Revision and Open Working File and Open Working File handles the renames and finds the correct working file.

I see... The current git extension doesn't really worry itself with anything other than HEAD, so this was never really addressed. Hmm. Let's leave this for GitLens for now.

Let's leave this issue open for the FileSystemProvider adoption.

@joaomoreno joaomoreno changed the title Add an optional workingPath to GitUriParams Adopt FileSystemProvider for Git Sep 24, 2018
@gjsjohnmurray
Copy link
Contributor

Might isomorphic-git be a path to getting VSCode's built-in git support to a place where it can play nice with FileSystemProvider? Apparently isomorphic-git is BYOFS (Bring Your Own File System).

@TomasHubelbauer
Copy link
Contributor

@wmhilton FYI the above suggestion. You can probably speak to this. :-)

@joaomoreno
Copy link
Member

@eamodio The git extension now uses the file system provider and a barebones utility has appeared in its API:

https://github.com/microsoft/vscode/blob/master/extensions/git/src/api/git.d.ts#L189

Needs more work in there, depending on on the requirements from extensions, but it's a starting step.

@eamodio
Copy link
Contributor Author

eamodio commented Nov 25, 2019

@joaomoreno Will we be supporting the directory traversal with the fs provider? That would enable breadcrumbs and workspace folder support.

@joaomoreno
Copy link
Member

joaomoreno commented Nov 26, 2019

@eamodio You mean creating a fs view in which only the modified files would exist? It's a cool idea, but it would require me to go away from encoding extra information in the uri query params and rely solely on path. There were a few issues wrt using path only, very early on, but those might've been ironed over the years. Maybe it's worthwhile looking into once again. Something like:

  • gitfs://workingtree/...
  • gitfs://index/...
  • gitfs://merge/...

@joaomoreno joaomoreno added the on-release-notes Issue/pull request mentioned in release notes label Dec 9, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality git GIT issues on-release-notes Issue/pull request mentioned in release notes on-testplan
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants