nonoroazoro / vscode-syncing

:zap: VSCode Extension - Sync all of your VSCode settings across multiple devices.
https://marketplace.visualstudio.com/items?itemName=nonoroazoro.syncing
Other
490 stars 30 forks source link

Respect Format On Save Settings on Download #33

Closed alok-mishra closed 6 years ago

alok-mishra commented 6 years ago

Specifications

Expected Behavior

When I download settings, do not format files or at-least respect formatOnSave settings.

Actual Behavior

When I download settings: settings.json gets formatted, even with all extensions disabled except for Syncing, and specific rules for not formatting jsonc.

Steps to Reproduce

  1. Add new lines / spaces between groups and blocks and on top of comments inside settings.json.

  2. Also add rules to disable formatting, including jsonc.

    "editor.formatOnSave": false,
    
    "[jsonc]": {
        "editor.formatOnSave": false
    },
    "[json]": {
        "editor.formatOnSave": false
    },
  3. Restart VS Code

  4. Then > Ctrl+Shift+P > "Syncing: Download Settings"

  5. View settings.json. It has been formatted to remove all spaces between blocks and above comments.

Personally I do want formatOnSave, but I would like it not to format settings.json so atleast the "[jsonc]" rule should be followed. Other formatting extensions and the built-in formatter respects this rule.

Thank you for this great extension.

nonoroazoro commented 6 years ago

@alok-mishra Did you enable the syncing.upload.exclude setting? If so, Syncing will try to format and merge you remote settings with local settings, because it uses a third-party lib to manipulate the JSON with comments, such as the VSCode Settings file.

nonoroazoro commented 6 years ago

@alok-mishra I checked the code and find a bug there... seems that Syncing will format the settings no matter the syncing.upload.exclude is enabled or not. I'll fix it in the next release. Thanks!

Fixed in https://github.com/nonoroazoro/vscode-syncing/commit/75444ff49c09a1088e9a1452cd3304e88435f0b4.

alok-mishra commented 6 years ago

@nonoroazoro Thank you very much! Yes I still want to sync my settings.json so can't exclude it.

In your edits, from what I understand, I think you are still formatting, only if the file has been modified.

Should there be another check to see if jsonc has been excluded from formatOnSave? Maybe I'm not understanding it, and it does what I need, but maybe something like this:

   if (modified && getFormatOnSave("jsonc")) {
         result = format(result);
   }
nonoroazoro commented 6 years ago

@alok-mishra That makes sense, the formatOnSave setting should also be checked before formatting.

nonoroazoro commented 6 years ago

@alok-mishra , I've released the new version (latest release) for this feature, enjoy!