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

Dont write default settings sync config to code settings.json #513

Closed
escape0707 opened this issue Mar 13, 2018 · 17 comments
Closed

Dont write default settings sync config to code settings.json #513

escape0707 opened this issue Mar 13, 2018 · 17 comments

Comments

@escape0707
Copy link

escape0707 commented Mar 13, 2018

Visual Studio Code Version : [ 1.21.0 ]
Code Settings Sync Version : [ 2.9.0]
Operating System : [ Windows 10]
Occurs On: [Upload / Download ]
Proxy Enabled: [ No ]
Gist Id: [ 9d1b34143f9769d9514e22c0e9757147]

image

First of all, THANK YOU for created such a helpful extension, it saved me many time!

How ever here is a little thing I think can get optimized.
The first part is the settings I customized or should be aware of.
The second part is the settings which are equal to default that the extension automatically inserted whenever Uploading / Downloading.
IMO the users shouldn't see and be aware of those "changes" as they don't make any differences and only to figure out the global settings were just polluted with many unnecessary lines.

I have posted frequently asked questions on
http://shanalikhan.github.io/2016/11/07/visual-studio-code-frequently-asked-questions.html
so you may able to resolve the error quickly.
Please make sure to write the versions and console logged error picture or copied text

@shanalikhan
Copy link
Owner

Nice idea
Will evaluate it

@escape0707 escape0707 changed the title Feature Requeset: Don't introduce "sync.*" settings which is equal to default behaviour after DL/UL Feature Request: Don't introduce "sync.*" settings which is equal to default behavior after DL/UL Mar 19, 2018
@shanalikhan
Copy link
Owner

  1. Last Download
  2. Last Upload

IMO are the keys thats best to shift from code user settings to settings sync custom settings file.
Besides that, all the other keys are somehow configurable by user.

@shanalikhan shanalikhan added this to the v3.1 milestone Jul 22, 2018
shanalikhan added a commit that referenced this issue Aug 14, 2018
@shanalikhan
Copy link
Owner

  1. Last Update - Extension auto update this key, its safe to transfer it to Settings Sync configuration
  2. Last Download - Extension auto update this key, its safe to transfer it to Settings Sync configuration

Following keys are be global in Settings Sync configuration rather then Code Configuration, as users for Github Enterprise they need to update these settings and than download the settings which replaces them.

  1. Host
  2. pathPrefix

shanalikhan added a commit that referenced this issue Aug 18, 2018
shanalikhan added a commit that referenced this issue Aug 26, 2018
* german localization

* corrected naming of locale

* german localization

* corrected naming of locale

* fix: missing partial i18n translation

* fix: missing partial i18n translation

* fix: add missing key for German language file

* fix: typo

* Minor Wording Changes

The text on some parts looked strange and not correct, thought I'd update a few for grammar and looks.

* chore: update .gitignore

* chore: add tslint and prettier rules

* chore: add format script

* chore: update teslint rules to make it defined mutilple class in one file

* chore: run tslint before publish extension

* refactor: format the code with prettier

* chore: update tsconfig and tslint

* refactor: refactor utils.ts

* refactor: refactor localize.ts

* refactor: refactor setting.ts

* refactor: refactor environmentPath.ts

* refactor: refactor fileService.ts

* chore: update teslint rules

* refactor: replace github package with @octokit/rest

* chore: update teslint rules

* refactor: refactor commons.ts

* refactor: refactor pluginService.ts

* refactor: extension.ts

* feat: replace fs module with fs-extra, and use async await instead of callback style

* feat: update deps for vscode/typescript/adm-zip/tslint

* refactor: refactor callback style with async await

* remove unused dependencies ncp and rimraf, use fs-extra instead

* fix: invalid @octokit/rest constructor options

* chore: update travis script

* fix: lstat should be async no sync

* fix: user agent for proxy

* pref: improve performance when start up. use async instead of sync method #472

* fix: implement promisify and fix lockfile

* feat: use lockfile instead of proper-lockfile.

* pref: improve performance

* refactor: add sync class and clean up extension file. it should improve startup performance #472

* refactor: add lockfile.ts

* fix: typo ShowSummaryOutput

* refactor: refactor download function

* fix: lockfile

* refactor: rename 'en' to 'env'

* refactor: remove unused activationEvents

* refactor: refactor upload logic, make it clean

* style: rename lockfile function name, make it same style with other

* refactor: sync advance options

* chore: not allow unuser locals and params. make it clean

* refactor: remove unused local and params

* refactor: remove unneed await

* pref: improve localize performance, init when extension be actived

* fix: make sure resolve language merger with default language.

* Update tutorial message

* Fix slack img

* Update path to imgur link

* Update tutorial link again

* Extension Installation CLI Added
TSLint Improved
#434
#590
#513
#337

* Summary improved

* Github Api Code Improvements

Anonymous Gist Code Removed
Initializtion in contructor
github api updated
proxy addition
github enterprise support updated

* Ignored extensions can be accidentally deleted if removeExtensions is enabled. (#604)

* Fix for #516

* Update to fix `ignoredExtensions` where extensions were deleted if `removeExtensions` was enabled.

* #604

* Extension summary

#577

* Gist Name change
#513

* #611

* fix: error translation

* #611 - Changelog

CLI Improvements

* #611

* #611
@thernstig
Copy link

@shanalikhan Maybe I misunderstood this issue and MR, but maybe you can clarify.

I just downloaded 3.1 and then tried to change my user settings to remove these lines:

    "sync.quietSync": false,
    "sync.removeExtensions": true,
    "sync.syncExtensions": true,
    "sync.forceDownload": false,

When I save and upload the settings, those configurations gets added to the bottom to my user settings. Was that not something this issue tried to fix?

@shanalikhan
Copy link
Owner

Yes these settings are gist specific settings, all other keys have been move to a part of global settings for Settings Sync.

@thernstig
Copy link

thernstig commented Aug 29, 2018

@shanalikhan Ok thanks for the clarification. I probably misunderstood the issue then. Since the 4 settings above are set to default values, I would have thought they would not show up in my own user settings.

The reason why I thought so is that any other extensions I have does not include default settings in the user/workspace settings.json. But I suppose it could be different here for how this plugin works as it syncs settings.

@ryenus
Copy link

ryenus commented Aug 30, 2018

Like @thernstig has pointed out, several settings are added back after uploading, though they have default values:

@thernstig
Copy link

@shanalikhan Did you see mine and @ryenus follow-up comment? Is there a reason why default settings are set in the "User Settings" part of VS Code?

@escape0707
Copy link
Author

So sorry for being absent for such a long time. And yes, what you guys pointed out is what I was thinking about when I post this issue. Haven't this got fixed now yet?

@shanalikhan
Copy link
Owner

These settings cant move outside from GIST for:

  1. These settings are the environment settings, a user can have multiple GIST ( Environment )
  2. Other settings that were not related to the user's environment (Single GIST) has been moved to Global Config that is shared across multiple environments.

@escape0707
Copy link
Author

I mean, if in a gist, the extension can't find these settings they can simply use the factory default behavior right?
And if you are afraid of the default settings being too aggressive like "remove extensions", then it's the default value should be changed and we shouldn't leave a setting there explicitly to warn users.
Hum, I hope I got your ideas right, if I'm missing something you can explain and clarify my confusion. I'll be grateful!

@shanalikhan
Copy link
Owner

I think I misunderstood your issue back all the time.
What are you asking to NOT WRITE sync keys inside the settings.json which have default values/behaviour?
If this is what you are asking let me know I will reopen this issue. :)

@thernstig
Copy link

thernstig commented Sep 4, 2018

@shanalikhan Yes, exactly! (I know I am speaking for @zzhhbyt1 here, but I am pretty sure that is what he means).

That is exectly how it works for any other VS Code extensions I have installed.

Just imagine if every extension did that, for example the Python extension which has 89 settings.

@shanalikhan shanalikhan reopened this Sep 4, 2018
@shanalikhan shanalikhan removed the fixed label Sep 4, 2018
@shanalikhan shanalikhan added this to the v3.2 milestone Sep 4, 2018
@escape0707
Copy link
Author

Lol, yeah you guys understand me now finally! XD I shall say in old Chinese guys style:"Sorry for my bad English." XDDDD

@shanalikhan shanalikhan changed the title Feature Request: Don't introduce "sync.*" settings which is equal to default behavior after DL/UL Dont write default settings sync config to code settings.json Sep 18, 2018
@thernstig
Copy link

thernstig commented Oct 11, 2018

@shanalikhan Did you get a chance to look at this? I dislike polluting my user with unnecessary settings 😄

@shanalikhan
Copy link
Owner

Yes, i might work on it in few weeks.
Anyone of you can send me a PR though 😄

shanalikhan added a commit that referenced this issue Oct 12, 2018
shanalikhan added a commit that referenced this issue Oct 12, 2018
shanalikhan added a commit that referenced this issue Oct 17, 2018
* chore(package): update tslint-plugin-prettier to version 2.0.0

* Updated: German translation

* #641

* Add support for VSCodium

* Corrected the message to be displayed

* Add Japanese translation file

* be able to upload additional file

* be able to download additional files

* create new advanced setting to add Customized Sync file

* fix typo eslint => eslintrc

* Add Import Custom File to workspace option

* unify display message: Custom Sync File

* Test Directory

* =Adding HostName property. Get hostname using 'os.hostname()'

* =Need definition for supported OS's. Using strict match regex to get @sync pragmas. Should it support multiple spaces? Replacing settings.json content before writing file. Should be better removing lines instead of add comments? Replacing settings.json before upload to remove @sync ignore pragms.

* Added English message for OSNotSupported message while processing uploading content

* mocha and chai packages added as dev dependencies. Test script not working while compiling.

* Moving pragma functions to a separated file

* Added Pragma util static class. Support for host, env and os values in every orther. Remove whitespaces before upload. Alert user if OS value is not a valid OS.

* Added hostName property to CustomSettings

* Tests files added. Estructure test per feature.

* Added some documentation. hostName property on custom config.

* uncomment lines function. Uncomment all @sync settings before upload. Only insert comments if it doesn't match with matchine os or host or env. Uncomment line before write if it matched.

* Do replace ments one time per setting pragma. Test uncoment function. More redeable settings json for testing.

* Add SUPPORTED_OS and osTypeFromString to environmentPath.ts as requested. Get OS from OsType enum. Remove os.hostName()

* Check valid JSON before writting file. All the comments and trilling commas are removed. Must check this. If not valid OS is detected inform user. Added function to remove comments from text.

* Chatch exception during upload. Should it abort upload?

* Adding VS Code tests in typescript. Run test changing launch mode to "Launch Test". Remove javascript files.

* Environment - treat non-OSS instalations on Linux as XDG

* Improvements
#628 , #629

* minor grammatical fixes

* add fileService.test.ts

* Version Change

* #331

* #513

* Add webpack bundling (#676)

* Update issue templates

* add webpack bundling for faster startup

* no conflicts

* Revert "Add webpack bundling (#676)" (#679)

This reverts commit e68a243.

* Gist Object changed

* Update package.json

* v3.2 change log
@shanalikhan
Copy link
Owner

Fixed in v3.2 at last.
Let me know if there is any problem

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

4 participants