search

microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.51k stars 29.39k forks source link

`settingsSync.keybindingsPerPlatform` not working on version 1.59.0 #130352

Closed thanhph111 closed 3 years ago

thanhph111 commented 3 years ago

Describe

I updated to version 1.59.0 on Ubuntu a few days ago, nothing happened. However, when I did it on Windows today, it shows a conflict trying to merge keybindings. I realized it was because the VSCode didn't recognize the differences between platforms when I tried to re-enable settingsSync.keybindingsPerPlatform (which was enabled at first).

Checklist

Steps to Reproduce:

  1. Check that the settings settingsSync.keybindingsPerPlatform is enabled.
  2. Add a new keyboard shortcut on Windows or Ubuntu (check if it's synced).
  3. Go to the other machine, check syncing, and that keybinding will appear.

Expected behaviour

VSCode should respect platform-specific keyboard shortcuts.

System info

pravusid commented 3 years ago

I've got the same problem. MacOS and Linux. 😭

System info

Version: 1.59.0
Commit: 379476f0e13988d90fab105c5c19e7abc8b1dea8
Date: 2021-08-04T23:14:40.191Z
Electron: 13.1.7
Chrome: 91.0.4472.124
Node.js: 14.16.0
V8: 9.1.269.36-electron.0
OS: Darwin x64 20.5.0
Version: 1.59.0
Commit: 379476f0e13988d90fab105c5c19e7abc8b1dea8
Date: 2021-08-04T23:13:20.182Z
Electron: 13.1.7
Chrome: 91.0.4472.124
Node.js: 14.16.0
V8: 9.1.269.36-electron.0
OS: Linux x64 5.10.56-1-MANJARO
mavenor commented 3 years ago

…And I’ve got the same problem between macOS and Windows☹️ Now that seals it, I think 😂

I had settingsSync.keybindingsPerPlatform set as well. The issue still remained after multiple spam-clicks, frustrated glares (at the checkbox), and a few hopeful restarts between the two OSes (why not).

System Info

macOS (Monterey (Public) Beta 5, 21A5304g) Version: 1.59.0
Commit: 379476f0e13988d90fab105c5c19e7abc8b1dea8
Date: 2021-08-04T23:14:40.191Z
Electron: 13.1.7
Chrome: 91.0.4472.124
Node.js: 14.16.0
V8: 9.1.269.36-electron.0
OS: Darwin x64 21.0.0
Windows 10 (21H1, Build 19043.1165) Version: 1.59.0 (system setup)
Commit: 379476f0e13988d90fab105c5c19e7abc8b1dea8
Date: 2021-08-04T23:13:12.822Z
Electron: 13.1.7
Chrome: 91.0.4472.124
Node.js: 14.16.0
V8: 9.1.269.36-electron.0
OS: Windows_NT x64 10.0.19043
mavenor commented 3 years ago

@thanhph111 I think that checks second box too, then (😄+☹️)

thanhph111 commented 3 years ago

@thanhph111 I think that checks second box too, then (😄+☹️)

Thanks for reminding me, checking the box is always the easy part 😆.

yo1dog commented 3 years ago

Also occurs for me between Windows and Mac.

Windows

Version: 1.59.1 (user setup) Commit: 3866c3553be8b268c8a7f8c0482c0c0177aa8bfa Date: 2021-08-19T11:56:46.957Z Electron: 13.1.7 Chrome: 91.0.4472.124 Node.js: 14.16.0 V8: 9.1.269.36-electron.0 OS: Windows_NT x64 10.0.19042

VS Code version: Code 1.59.1 (3866c3553be8b268c8a7f8c0482c0c0177aa8bfa, 2021-08-19T11:56:46.957Z) OS version: Windows_NT x64 10.0.19042 Restricted Mode: No Remote OS version: Linux x64 5.4.72-microsoft-standard-WSL2 Remote OS version: Linux x64 5.4.72-microsoft-standard-WSL2 Remote OS version: Linux x64 5.4.72-microsoft-standard-WSL2 Remote OS version: Linux x64 5.4.72-microsoft-standard-WSL2

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i9-10850K CPU @ 3.60GHz (20 x 3600)| |GPU Status|2d_canvas: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
skia_renderer: enabled_on
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled| |Load (avg)|undefined| |Memory (System)|31.85GB (16.02GB free)| |Process Argv|--crash-reporter-id 9d5d78ac-4897-44fe-912c-31d69a4a19c4| |Screen Reader|no| |VM|0%| |Item|Value| |---|---| |Remote|WSL: Ubuntu| |OS|Linux x64 5.4.72-microsoft-standard-WSL2| |CPUs|Intel(R) Core(TM) i9-10850K CPU @ 3.60GHz (20 x 3599)| |Memory (System)|24.94GB (19.66GB free)| |VM|0%| |Item|Value| |---|---| |Remote|WSL: Ubuntu| |OS|Linux x64 5.4.72-microsoft-standard-WSL2| |CPUs|Intel(R) Core(TM) i9-10850K CPU @ 3.60GHz (20 x 3599)| |Memory (System)|24.94GB (19.66GB free)| |VM|0%| |Item|Value| |---|---| |Remote|WSL: Ubuntu| |OS|Linux x64 5.4.72-microsoft-standard-WSL2| |CPUs|Intel(R) Core(TM) i9-10850K CPU @ 3.60GHz (20 x 3599)| |Memory (System)|24.94GB (19.66GB free)| |VM|0%| |Item|Value| |---|---| |Remote|WSL: Ubuntu| |OS|Linux x64 5.4.72-microsoft-standard-WSL2| |CPUs|Intel(R) Core(TM) i9-10850K CPU @ 3.60GHz (20 x 3599)| |Memory (System)|24.94GB (19.66GB free)| |VM|0%|
Extensions (10) Extension|Author (truncated)|Version ---|---|--- remote-ssh|ms-|0.65.7 remote-ssh-edit|ms-|0.65.7 remote-wsl|ms-|0.58.2 markdown-preview-github-styles|bie|0.2.0 vscode-eslint|dba|2.1.23 gitlens|eam|11.6.0 vscodeintellicode|Vis|1.2.14 cursor-align|yo1|1.0.4 cursor-trim|yo1|1.0.0 multi-cursor-search|yo1|2.0.4
A/B Experiments ``` vsliv368cf:30146710 vsreu685:30147344 python383:30185418 pythonvspyt602:30300191 vspor879:30202332 vspor708:30202333 vspor363:30204092 pythonvspyt639:30300192 pythontb:30283811 pythonptprofiler:30281270 vshan820:30294714 vstes263:30335439 pythondataviewer:30285071 pythonvsuse255:30340121 vscod805cf:30301675 pythonvspyt200:30340761 vscextlangct:30333562 binariesv615:30325510 pythonvssor306:30344512 bridge0708:30335490 pygetstartedc2:30351386 vstre464cf:30350173 bridge0723:30353136 vsdyn478:30354110 ```

Mac

Version: 1.59.0 Commit: 379476f0e13988d90fab105c5c19e7abc8b1dea8 Date: 2021-08-04T23:14:40.191Z (2 wks ago) Electron: 13.1.7 Chrome: 91.0.4472.124 Node.js: 14.16.0 V8: 9.1.269.36-electron.0 OS: Darwin x64 19.6.0

System Info CPUs | Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz (8 x 2600) -- | -- GPU Status | 2d_canvas: enabled gpu_compositing: enabled metal: disabled_off multiple_raster_threads: enabled_on oop_rasterization: enabled opengl: enabled_on rasterization: enabled skia_renderer: disabled_off_ok video_decode: enabled webgl: enabled webgl2: enabled Load (avg) | 1, 2, 2 Memory (System) | 16.00GB (1.59GB free) Process Argv | . --disable-extensions --crash-reporter-id 8f89bf5e-113f-4689-8763-03731b04cf02 Screen Reader | no VM | 0%
Extensions (10) Extension | Author (truncated) | Version -- | -- | -- markdown-preview-github-styles | bie | 0.2.0 gitlens | eam | 11.6.0 vscode-postgresql | ms- | 0.3.0 remote-ssh | ms- | 0.65.7 remote-ssh-edit | ms- | 0.65.7 remote-wsl | ms- | 0.58.2 vscodeintellicode | Vis | 1.2.14 cursor-align | yo1 | 1.0.4 cursor-trim | yo1 | 1.0.0 multi-cursor-search | yo1 | 2.0.4
A/B Experiments ``` vsliv368cf:30146710 vsreu685:30147344 python383cf:30185419 pythonvspyt602:30300191 vspor879:30202332 vspor708:30202333 vspor363:30204092 vstes627:30244334 pythonvspyt639:30300192 pythontb:30283811 pythonvspyt551:30345470 pythonptprofiler:30281270 vshan820:30294714 vstes263cf:30335440 pythondataviewer:30285071 pythonvsuse255:30340121 vscod805:30301674 pythonvspyt200:30340761 vscextlangct:30333562 binariesv615:30325510 pythonvssor306:30344512 bridge0708:30335490 vstre464:30350172 bridge0723:30353136 ```
yo1dog commented 3 years ago

As a workaround, I combined my Windows and Mac keybindings and added a when "when": "isWindows" and "when": "isMac" to each binding.

sandy081 commented 3 years ago

@lramos15 Unfortunately this is caused by the following change - https://github.com/microsoft/vscode/commit/a08a485a914d9a221d198c13a8208ca9f851cc31#diff-aa2d2ba960a74c22e50720f94dc2fbd26020905ef1692a8c37e2d7099d80651fR334. This switched off settingsSync.keybindingsPerPlatform by default, which means keyboard shortcuts are synced across platforms by default.

Fixed it. But please aware that this will cause conflicts for existing users when syncing keyboard shortcuts because the default value is changed to sync per platform and therefore sync will ask user to accept right changes.

Note: You can safely accept the merge when you are asked to resolve conflicts because of this.

lramos15 commented 3 years ago

@lramos15 Unfortunately this is caused by the following change - a08a485#diff-aa2d2ba960a74c22e50720f94dc2fbd26020905ef1692a8c37e2d7099d80651fR334. This switched off settingsSync.keybindingsPerPlatform by default, which means keyboard shortcuts are synced across platforms by default.

Oh no, sorry! This was part of my work to try and slowly transition the configuration service to not be templated for better runtime type safety and I just assumed !! would return the same boolean value and would be safe in converting non boolean values to boolean ones

yo1dog commented 3 years ago

The !! boolean conversion is not the issue. If I am reading the code correctly, the issue is you changed the type from boolean | undefined to boolean. Specifically, by converting settingsSync.keybindingsPerPlatform from undefined to false it now acts as if it is explicitly set to false, rather than being treated as unset, and prevents the default value of true from being used (the !== undefined check will always return true).

Aside: This could be more simply written using the nullish coalescing operator:

return Boolean(
  this.configurationService.inspect('settingsSync.keybindingsPerPlatform').userValue ??
  this.configurationService.inspect('sync.keybindingsPerPlatform').userValue ??
  this.configurationService.getValue('settingsSync.keybindingsPerPlatform')
);

Also, this means there is a simple workaround: Explicitly set "settingsSync.keybindingsPerPlatform": true in your user settings JSON file.