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

UriDisplayService is missing remote share path name #58222

Closed
sbatten opened this Issue Sep 7, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@sbatten
Member

sbatten commented Sep 7, 2018

Issue Type: Bug

  1. Open file on remote smb share in VS Code. e.g. \mysmb\folder\file.txt
  2. Hover over editor tab or right click > copy path for said file.

Expected path:
\\mysmb\folder\file.txt

Actual path:
\folder\file.txt

VS Code version: Code - Insiders 1.28.0-insider (a3f9f2e, 2018-09-07T05:20:29.346Z)
OS version: Windows_NT x64 10.0.17134

System Info
Item Value
CPUs Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz (8 x 2112)
GPU Status 2d_canvas: enabled
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: disabled_software
rasterization: enabled
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled
Memory (System) 15.93GB (7.10GB free)
Process Argv C:\Users\stbatt\AppData\Local\Programs\Microsoft VS Code Insiders\Code - Insiders.exe
Screen Reader no
VM 0%
@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Sep 18, 2018

Contributor

The issue here was that I was ignoring the authority when registering the file formatter.
On remote share paths the authority is the first part of the path which should be displayed.
For local files the authority is empty so this case should also be covered.
uri.fsPath was always doing this here
fyi @bpasero

Contributor

isidorn commented Sep 18, 2018

The issue here was that I was ignoring the authority when registering the file formatter.
On remote share paths the authority is the first part of the path which should be displayed.
For local files the authority is empty so this case should also be covered.
uri.fsPath was always doing this here
fyi @bpasero

@isidorn isidorn closed this in 42c0dd5 Sep 18, 2018

@msftrncs

This comment has been minimized.

Show comment
Hide comment
@msftrncs

msftrncs Sep 23, 2018

@isidorn , not sure this is fixed yet. Using:

Version: 1.28.0-insider (user setup)
Commit: 2558a72d6902015b142ef45c57b664ce1f09e03f
Date: 2018-09-21T05:19:52.215Z
Electron: 2.0.9
Chrome: 61.0.3163.100
Node.js: 8.9.3
V8: 6.1.534.41
Architecture: x64

I now see the server name when the path to files are shown, but if I copy the path, the path reads without the leading \\ so its still not a complete path. Also, maybe because of this, the 'compare with saved' still doesn't work for files on UNC paths.

msftrncs commented Sep 23, 2018

@isidorn , not sure this is fixed yet. Using:

Version: 1.28.0-insider (user setup)
Commit: 2558a72d6902015b142ef45c57b664ce1f09e03f
Date: 2018-09-21T05:19:52.215Z
Electron: 2.0.9
Chrome: 61.0.3163.100
Node.js: 8.9.3
V8: 6.1.534.41
Architecture: x64

I now see the server name when the path to files are shown, but if I copy the path, the path reads without the leading \\ so its still not a complete path. Also, maybe because of this, the 'compare with saved' still doesn't work for files on UNC paths.

@msftrncs

This comment has been minimized.

Show comment
Hide comment
@msftrncs

msftrncs Sep 23, 2018

the error message I get, when comparing with saved actually still fails to show the server name, so this may be a different but maybe related issue:

Unable to open 'TheBigTestFile.ps1 (on disk) ↔ TheBigTestFile.ps1': File not found (file:///User Documents/Carl/PowerShell Scripting/VS Code Language Syntax/sample scripts/TheBigTestFile.ps1).

User Documents is the share name, the server name is completely gone.

msftrncs commented Sep 23, 2018

the error message I get, when comparing with saved actually still fails to show the server name, so this may be a different but maybe related issue:

Unable to open 'TheBigTestFile.ps1 (on disk) ↔ TheBigTestFile.ps1': File not found (file:///User Documents/Carl/PowerShell Scripting/VS Code Language Syntax/sample scripts/TheBigTestFile.ps1).

User Documents is the share name, the server name is completely gone.

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Sep 24, 2018

Contributor

@sbatten can you please verify if this is fixed
@msftrncs compare with is an unrelated bugs, please file new issue. Copy path should I will look into fixing

Contributor

isidorn commented Sep 24, 2018

@sbatten can you please verify if this is fixed
@msftrncs compare with is an unrelated bugs, please file new issue. Copy path should I will look into fixing

@sbatten

This comment has been minimized.

Show comment
Hide comment
@sbatten

sbatten Sep 24, 2018

Member

@isidorn it now shows the full path but in the form "mysmb\folder\file.txt" which is strange because you can't copy and cd to that path. I would expect the path to maintain the leading\\

Member

sbatten commented Sep 24, 2018

@isidorn it now shows the full path but in the form "mysmb\folder\file.txt" which is strange because you can't copy and cd to that path. I would expect the path to maintain the leading\\

@ThubLives

This comment has been minimized.

Show comment
Hide comment
@ThubLives

ThubLives Sep 26, 2018

This doesn't seem to be fixed, only slightly less broken. I just tried with the Insider build and I get server\share\file.ext -- which is not a valid UNC path -- instead of \\server\share\file.ext. Should this be issue be re-opened?

ThubLives commented Sep 26, 2018

This doesn't seem to be fixed, only slightly less broken. I just tried with the Insider build and I get server\share\file.ext -- which is not a valid UNC path -- instead of \\server\share\file.ext. Should this be issue be re-opened?

@sbatten

This comment has been minimized.

Show comment
Hide comment
@sbatten

sbatten Sep 27, 2018

Member

@ThubLives the latest fix is not yet available in insiders. Should be in the next build.

Member

sbatten commented Sep 27, 2018

@ThubLives the latest fix is not yet available in insiders. Should be in the next build.

@msftrncs

This comment has been minimized.

Show comment
Hide comment
@msftrncs

msftrncs Sep 27, 2018

Got the latest insiders, path now shows server with \\, copy path includes it as well. 'compare with saved' was not affected by this fix.

msftrncs commented Sep 27, 2018

Got the latest insiders, path now shows server with \\, copy path includes it as well. 'compare with saved' was not affected by this fix.

@isidorn

This comment has been minimized.

Show comment
Hide comment
@isidorn

isidorn Sep 27, 2018

Contributor

@msftrncs thanks for trying it out, adding verified label

Contributor

isidorn commented Sep 27, 2018

@msftrncs thanks for trying it out, adding verified label

@isidorn isidorn added the verified label Sep 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment