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.

Sign up for GitHub

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

Jump to bottom

ColorTool does not resolve schemes properly, and does not work with WSL paths #5326

Closed
johnazariah opened this issue Apr 12, 2020 · 3 comments · Fixed by #5327
Closed

ColorTool does not resolve schemes properly, and does not work with WSL paths #5326

johnazariah opened this issue Apr 12, 2020 · 3 comments · Fixed by #5327
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Colortool This is a problem with the utility application ColorTool for manipulating the console color palette. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Console Backlog

Comments

@johnazariah
Copy link
Member

Environment

10.0.18363.0

Windows build number: [run `[Environment]::OSVersion` for powershell, or `ver` for cmd]
Windows Terminal version (if applicable):

Any other software?

Ubuntu on WSL

Steps to reproduce

  • on Windows Command Shell/PowerShell/GitBash
    • running ColorTool.exe <schemename> (without extension) does not resolve the scheme
    • running ColorTool.exe <schemename.extension> resolves the scheme correctly.

This behaviour is contrary to the advertised behaviour, which says that the scheme will be resolved trying a specific order of extensions

  • on Ubuntu bash running in WSL
    • running ColorTool.exe -x <schemename> (without extension) does not resolve the scheme
    • running ColorTool.exe -x <schemename.extension> does not resolve the scheme and throws exceptions.

This behaviour is also not the expected behaviour.

Expected behavior

ColorTool.exe OneHalfDark should have the same behaviour as ColorTool.exe OneHalfDark.itermcolors, and should actually load the terminal colors on both Windows shells and WSL shells

Actual behavior

before
before-wsl
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 12, 2020
@DHowett-MSFT
Copy link
Contributor

It looks like this might be because your color schemes are in the WSL-only part of the filesystem. We haven’t yet done the compatibility work required to make sure ColorTool can use the new \\wsl$ redirector paths from Windows 19H1.

@DHowett-MSFT DHowett-MSFT changed the title ColorTool does not resolve schemes properly, and does not work on WSL ColorTool does not resolve schemes properly, and does not work with WSL paths Apr 12, 2020
@johnazariah johnazariah mentioned this issue Apr 12, 2020
Merged
@ghost ghost added the In-PR This issue has a related PR label Apr 12, 2020
@johnazariah
Copy link
Member Author

Thanks, @DHowett-MSFT

I've put it a fix that works in my test cases...

#5327
after
after-wsl

Hope this helps :)

@DHowett-MSFT
Copy link
Contributor

Thanks for submitting a fix! We're a little behind on pull request reviews right now, but I'll tag this bug up properly in the meantime.

@DHowett-MSFT DHowett-MSFT added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Colortool This is a problem with the utility application ColorTool for manipulating the console color palette. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 17, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 17, 2020
@DHowett-MSFT DHowett-MSFT added this to the Console Backlog milestone Apr 17, 2020
@DHowett-MSFT DHowett-MSFT mentioned this issue May 3, 2020
Closed
@ghost ghost closed this as completed in #5327 Jul 1, 2020
ghost pushed a commit that referenced this issue Jul 1, 2020
@johnazariah
fix scheme name resolution, and schema load on WSL (#5327)
436fac6 
This PR fixes the scheme resolution bug outlined in #5326

The approach is as follows:

* In [SchemeManager.cs], find the first scheme parser that actually
  successfully parses the scheme, as opposed to the existing code, which
  finds the first scheme parser which _says it can parse the scheme_, as
  that logic spuriously returns `true` currently. 
* In [XmlSchemeParser.cs] and [JsonParser.cs], ensure that the contents
  of the file are read and the contents passed to XmlDocument.LoadXXX,
  as this fails with an UriException on WSL otherwise.
* Remove `CanParse` as it is superfluous. The check for a valid scheme
  parser should not just check an extension but also if the file exists
  - this is best done by the `ParseScheme` function as it already
  returns null on failure.
* Add `FileExtension` to the interface because we need it lifted now.

Closes #5326
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jul 1, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Colortool This is a problem with the utility application ColorTool for manipulating the console color palette. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Console Backlog
Development

Successfully merging a pull request may close this issue.

fix scheme name resolution, and schema load on WSL
2 participants
@johnazariah @DHowett-MSFT