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

Use platform specific default value for separator #128348

Closed

Conversation

IllusionMH
Copy link
Contributor

This PR fixes #128345

This PR naively sets explicit default value that should match current behavior on Windows.

P.S. @bpasero I tried to use Copy Relative Path in web version (lauched as yarn web as I don't have access to Codespaces), however it always copies with \\ independently from setting. Is it expected behavior or I should file new issue for web version?

@bpasero
Copy link
Member

bpasero commented Jul 10, 2021

@IllusionMH we cannot set a default that is platform specific because a windows box might be connected to a linux remote. We only know the standard path separator at the time we copy the path by checking the remote OS.

@IllusionMH
Copy link
Contributor Author

@bpasero agree. Looks like I've missed that VS Code correctly copies path on Windows when connected to WSL or Remote now so platform specific check doesn't make sense.

What about question about yarn web where it always copies e.g. \sample-folder\large\rnd.foo for both Copy Path and Copy Relative Path and doesn't account for "explorer.copyRelativePathSeparator": "/" at all? It is expected behavior?

@isidorn isidorn removed their assignment Jul 28, 2021
@bpasero
Copy link
Member

bpasero commented Aug 2, 2021

@IllusionMH good catch, looks like we fail to use the separator if we cannot find a formatting:

if (formatting && options.separator) {
// mixin separator if defined from the outside
formatting = { ...formatting, separator: options.separator };
}

Do you want to change this PR to fix that particular issue?

As to why we use backslash by default in web on Windows, I refer to @isidorn who owns the label service.

@isidorn
Copy link
Contributor

isidorn commented Aug 2, 2021

backslash by default in web on Windows

I think we just use backslash by default on Windows. We did not write code for the web specific scenario.
But I think it might make sense if the use is copying paths from web to his local windows machine.

@IllusionMH
Copy link
Contributor Author

copying paths from web to his local windows machine

@isidorn What are current expected workflows for "web" version? My assumption was that "web" version is mainly for Codespaces that connects to Linux under the hood and has self sufficient development environments/workflows that won't need local PC and local paths.
Is it wrong assumption?

@bpasero I can take a stab on it or at least will create separate issue to track this unexpected behavior for "web" builds and will close this PR as you already properly fixed #128345 .

@bpasero
Copy link
Member

bpasero commented Aug 2, 2021

Sounds good thanks.

@bpasero bpasero closed this Aug 2, 2021
@IllusionMH IllusionMH deleted the default-separator-value-128345 branch August 26, 2021 19:19
@github-actions github-actions bot locked and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default presentation of Explorer: Copy Relative Path Separator in Setting editor is confusing
3 participants