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

Fixes #1045 - Take UNC path from query instead of uri #1209

Merged
merged 2 commits into from
Nov 30, 2020

Conversation

mikes-gh
Copy link

Description

When UNC path is used for a repo the gitUri is formatted without the double slash.

The following example is from a repo on //aesrocc/home/ROCCTEST

gitlens://0cee44d/aesrocc/home/ROCCTEST/fldedit/invfe53gst.et?{"path":"//aesrocc/home/ROCCTEST/fldedit/invfe53gst.et","ref":"0cee44d88243c4180f17837d9adf3b43a231ead2","repoPath":"//aesrocc/home/ROCCTEST"}
\_____/   \_____/\__________________________________________/ \___________________________________________________________________________________________________________________________________________/ 
   |        |            |                                                                                 |
scheme     authority       path                                                                 query   

The uri strips the //

uri.path = /aesrocc/home/ROCCTEST/fldedit/invfe53gst.et

but the path is still correct in the path element of the query string and should be reliable on all file systems

data.path = //aesrocc/home/ROCCTEST/fldedit/invfe53gst.et (from query)

Checklist

  • I have followed the guidelines in the Contributing document
  • My changes follow the coding style of this project
  • My changes build without any errors or warnings
  • My changes have been formatted and linted
  • My changes include any required corresponding changes to the documentation
  • My changes have been rebased and squashed to the minimal number (typically 1) of relevant commits
  • My changes have a descriptive commit message with a short title, including a Fixes $XXX - or Closes #XXX - prefix to auto-close the issue that your PR addresses

@mikes-gh
Copy link
Author

Probably also fixes #1087

@eamodio
Copy link
Member

eamodio commented Nov 30, 2020

@mikes-gh I put a patch in for this, although I think it is more of a band-aid than the root cause. I feel like the root cause is in here: https://github.com/eamodio/vscode-gitlens/blob/67a2c92f7967475e26df2afefc59ee2d44229923/src/git/gitUri.ts#L462-L510

Can you see if that is where the leading // is getting missed?

@eamodio eamodio self-assigned this Nov 30, 2020
@eamodio eamodio added this to the Soon™ milestone Nov 30, 2020
@eamodio eamodio merged commit f78c11b into gitkraken:main Nov 30, 2020
@mikes-gh
Copy link
Author

Yes I already debugged that area. The UNC path is correct there its further on in the process.
I am going to look at fsProvider.ts next

@mikes-gh
Copy link
Author

It seems the path is good with double slash upto
https://github.com/eamodio/vscode-gitlens/blob/b2859eccad6d4a3a7916e0f312e42392e6b51e52/src/commands/diffWith.ts#L169
When vscode.diff is run it calls back into the gitlens fsprovider and the uris somehow now miss the double slash??
Maybe the uri is rationalised in between.

@mikes-gh mikes-gh deleted the Bug-UNC branch November 30, 2020 20:54
@eamodio eamodio removed this from the Soon™ milestone Dec 2, 2020
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.

None yet

2 participants