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

Multi-line settings aren't ignored properly by sync #701

Closed
connorshea opened this issue Oct 29, 2018 · 8 comments
Closed

Multi-line settings aren't ignored properly by sync #701

connorshea opened this issue Oct 29, 2018 · 8 comments

Comments

@connorshea
Copy link

connorshea commented Oct 29, 2018

🐛 Describe the bug
When using the new per-platform sync feature, settings that run for more than one line aren't commented-out properly.

🌴 Visual Studio Code Version : Code 1.28.2 (7f3ce96ff4729c91352ae6def877e59c561f4850)
🌴 Code Settings Sync Version : 3.2.0
🌴 Standard or Insiders : Standard
🌴 OSS or Portable : neither?
🌴 Operating System : macOS Mojave 10.14.0
🌴 Occurs On: Download
🌴 Proxy Enabled: No

📰 To Reproduce
Steps to reproduce the behavior:

  1. Have VS Code with sync set up on two different machines (can be the same OS, just use the host option instead of os), we'll call them Machine A and Machine B.
  2. Create a user settings file on Machine A (in my case, Mac) like so:
{
  // @sync os=mac
  "editor.quickSuggestions": {
    "other": true,
    "comments": true,
    "strings": true
  }
}
  1. Sync the settings file to Machine B (Windows for me)
  2. This is what you'll end up with on Machine B:
{
  // @sync os=mac
  // "editor.quickSuggestions": {
    "other": true,
    "comments": true,
    "strings": true
  }
}

💪 Expected behavior
The setting should be properly commented-out on Machine B.

{
  // @sync os=mac
  // "editor.quickSuggestions": {
  //  "other": true,
  //  "comments": true,
  //  "strings": true
  // }
}
@shanalikhan
Copy link
Owner

shanalikhan commented Oct 30, 2018

Thanks! Will look into it.

@ioprotium
One more to go 😄

@protiumx
Copy link
Contributor

protiumx commented Nov 1, 2018

I will add that tomorrow! PR to 3.2.1 right?

@shanalikhan
Copy link
Owner

shanalikhan commented Nov 1, 2018 via email

@shanalikhan
Copy link
Owner

@ioprotium any update on this? :)

@protiumx
Copy link
Contributor

@shanalikhan Sorry for the delay. Yes I changed the code that parses the settings, I didn't contemplate the case for multi line settings, my bad. I will test on my environment and then drop the PR

@shanalikhan
Copy link
Owner

No problem - Thanks - Looking forward :)

@shanalikhan shanalikhan added this to the Backlog milestone Nov 27, 2018
@protiumx protiumx mentioned this issue Jan 14, 2019
@shanalikhan shanalikhan modified the milestones: Backlog, v3.2.5 Jan 15, 2019
@shanalikhan
Copy link
Owner

Fixed in v3.2.5
@connorshea

@connorshea
Copy link
Author

Thanks!

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

No branches or pull requests

3 participants