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

Ignore Upload some settings information in GIST #232

Closed
borekb opened this issue Feb 21, 2017 · 34 comments
Closed

Ignore Upload some settings information in GIST #232

borekb opened this issue Feb 21, 2017 · 34 comments

Comments

@borekb
Copy link

borekb commented Feb 21, 2017

Originally from #100 (comment)

Since #100, local settings can be replaced by replaceCodeSettings. However, this doesn't influence upload direction at all and if something changes frequently, there will be lots of new versions in the Gist for no good reason.

It would be good if replaceCodeSettings could also be taken into account during upload and replace the real value with some placeholder or something.

@rainbyte
Copy link

I think this would be useful, e.g.:

  • desktop vscode usually has "window.zoomLevel": 2
  • on my notebook I prefer "window.zoomLevel": 0

It would be better to just ignore that setting.

@shanalikhan
Copy link
Owner

@rainbyte
For now, On desktop you need to set
"replaceCodeSettings": {
{ "name :"window.zoomLevel",
"value":2
}
}
and for notebook , same way

@rainbyte
Copy link

Thanks @shanalikhan, that's what I'm doing now, it just doesn't feel right.

I would prefer to ignore the setting globally, without config per machine, if it is possible.

@shanalikhan shanalikhan added this to the v2.5.2 milestone Feb 24, 2017
@fredrikaverpil
Copy link

fredrikaverpil commented Feb 27, 2017

@borekb @shanalikhan if I read this right, you want any replacement code to be removed from the settings.json in the gist.

This would be nice, I agree, but please do not remove comments in settings.json which may contain these strings. I keep all of my different machine setups in the main settings.json file as comments.

@borekb
Copy link
Author

borekb commented Feb 28, 2017

I wouldn't remove the value from the uploaded JSON, I would just replace it with some known, single value.

For example, say that some settings stores a timestamp. Currently, if I upload the settings every second, I'll get many revisions of my settings.json with mess like this:

Gist v1:
option.timestamp: 1488289783

Gist v2:
option.timestamp: 1488289784

Gist v3:
option.timestamp: 1488289785

The goal would be to replace the ever-changing value with a single, non changing value, e.g., 1488289783 (it doesn't really matter). Now, when I invoke Upload settings every second, I won't get new versions of settings.json in my Gist.

@shanalikhan
Copy link
Owner

@borekb which setting key is changing, can you explain the real example of any extension of anything that cause change in values in settings.json again and again

@borekb
Copy link
Author

borekb commented Feb 28, 2017

@shanalikhan for me, it's window.zoomLevel. I change it frequently.

@shanalikhan shanalikhan modified the milestones: Backlog, v2.6.0 Mar 11, 2017
@shanalikhan shanalikhan changed the title Apply replaceCodeSettings also during upload Ignore Upload some settings information in GIST Apr 4, 2017
shanalikhan pushed a commit that referenced this issue Apr 7, 2017
Fixing #232
and adding quiet sync option
shanalikhan added a commit that referenced this issue Apr 18, 2017
Default Settings addition in custom settings
#295
#232
@shanalikhan shanalikhan reopened this Apr 22, 2017
@shanalikhan shanalikhan modified the milestones: v2.7, Backlog Apr 29, 2017
@shanalikhan
Copy link
Owner

I have released v2.7 adding this feature.
Let me know if there is any error, i will fix and release new version with documentation.
Or if its working fine 😄

@borekb
Copy link
Author

borekb commented May 3, 2017

Excellent! How does it work? I see a new ignoreUploadSettings key in syncLocalSettings.json, do I put some fixed value there and some regex will replace it in the uploaded settings.json? Thanks.

@shanalikhan
Copy link
Owner

shanalikhan commented May 3, 2017

For now , some fixed value !

like replaceCodeSettings

Oh i have posted the ignoreUploadSettings in read me also
Check that

@borekb
Copy link
Author

borekb commented May 3, 2017

I tried this in syncLocalSettings.json:

"ignoreUploadSettings": {
    "window.zoomLevel": 3
},
"replaceCodeSettings": {
    "window.zoomLevel": 2
},

This should mean, "when uploading to Gist, put 3 as the zoom level; when downloading, replace the value from Gist to 2". But I end up in 3 in the zoom property after download which feels incorrect. Also, it moved the zoomLevel it in my settings.json from top to bottom, not sure why.

@borekb
Copy link
Author

borekb commented May 3, 2017

Oh I see, ignoreUploadSettings removes the value from settings.json sent to Gist and applies the value when downloaded. Not sure how that fits with replaceCodeSettings; it seems that there is some overlap between the two.

@shanalikhan
Copy link
Owner

shanalikhan commented May 3, 2017

for your case, u only need one thing
Remove the replaceCodeSettings child.

"ignoreUploadSettings": {
    "window.zoomLevel": 3
}

Any value in window.zoomLevel will replace 3 and then remove from settings, upload and then get the value from there and put in settings.

There was no other way to do that.

I'm closing this issue, let me know if there is any problem

@borekb
Copy link
Author

borekb commented May 8, 2017

I either still don't understand how this is supposed to work, or it doesn't work correctly.

I have this in my syncLocalSettings.json:

"ignoreUploadSettings":{"window.zoomLevel":1}

and this in my settings.json:

"window.zoomLevel": 1

Now I close VSCode. The next time it starts, it downloads settings from Gist due to this configuration:

"sync.autoDownload": true,
"sync.autoUpload": false,

After this, I am left with a broken window zoom as it is completely missing from settings.json.

What removed it from there? I guess it's still not clear to me when exactly are ignoreUploadSettings applied; seems like not only during the upload because I was not uploading and it still removed window.zoomLevel from settings.json.

@swarnendubiswas
Copy link

@shanalikhan Did you hear back from upstream about the bug? I experience the same issue, I specify the list of keys to exclude, but they keep appearing in the gist.

@evenfrost
Copy link

evenfrost commented Dec 23, 2017

Same for me. I've set

  "ignoreUploadSettings": ["editor.fontSize"],

in syncLocalSettings.json but it still appears in the gist.

@evenfrost
Copy link

evenfrost commented Dec 26, 2017

SO, what's current workflow to keep some settings local? E.g. I have "editor.fontSize": 14 in synced settings which are uploaded to a gist, but want to set "editor.fontSize": 15 on different machine. What should I set then?
I've tried to use different combinations of "replaceCodeSettings" and "ignoreUploadSettings" fields in my "syncLocalSettings.json", but nothing seems to work, font is still shows as 14px.

@syepes
Copy link

syepes commented Jan 11, 2018

+1

@robertpeteuil
Copy link

robertpeteuil commented Feb 1, 2018

@swarnendubiswas @evenfrost @syepes - I dont have much good news to share. I've ran into a similar issue and have spent a while research it to no avail.

I can tell you that the upstream issues he mentioned about have been closed. One was flagged by-design and they second was auto-closed in July for "needs more information and has not had recent activity".

Also - ignoreUploadSettings has been removed, according to the commits listed above. So the only options left is replaceCodeSettings. It doesn't work correctly for my scenario, and it seems like the extension owner is focused elsewhere for the time being.

@shanalikhan
Copy link
Owner

shanalikhan commented Mar 4, 2018

@robertpeteuil Dont worry! 😄

I'm the only one working and maintaining.
Hard to manage Open Source, Office and Study! ( Looking forward for contributors )

I worked on this feature and after reaching toward end , found some show stopping Code API limitations so i had to revert it.

Released v2.9 today with alot of features.
I will release v3.0 with this fixed soon.

Update : I have to look when its possible to work on it :)

@robertpeteuil
Copy link

robertpeteuil commented Mar 5, 2018

@shanalikhan Awesome, thank you.

I understand the challenge of maintaining open source, while juggling other responsibilities.

There was a specific issue I was having, which I believe is (or at least was) related to the logic of how it processes ignored local settings.

  • Specifically, if an ignored setting has a value of false, it assumes that it can be removed from the local settings.
  • This is an incorrect assumption as there are settings that default to True which require explicit False values.
    • In these cases, deleting the setting is the same as setting it to True

An example - I was trying to hide the Docker Explorer on 2 of machines but allow it to be shown on a 3rd. (It's displayed by default, unless there's a setting that explicitly sets it to False)

  • I set it to ignore changes of the setting docker-show-explorer (made up setting name)
  • on the machines where I wanted it hidden I set docker-show-explorer to False
  • on the machine where I wanted it shown I set docker-show-explorer to True
  • during a sync, the setting is deleted on the machines where it was set to False
    • as mentioned above, this is bad, as it's the same as setting it to True.
  • I tested this by changing the values of these settings to both True and string values.
    • In these cases the sync properly kept the local values.
    • It's only the False value that causes this un-wanted behavior.

I hope I explained this properly. If not, let me know and I'll provide more details.

@shanalikhan shanalikhan modified the milestones: v3.0, Backlog Jul 11, 2018
@spali
Copy link

spali commented Aug 31, 2018

Hi
Just stumbled about this.
Just my two cents about the settings replaceCodeSettings. For my and the example use case with the proxy it works in general.
But what I don't like in this way (filter/set during download settings) for example the proxy settings (even it's not confidential), it should never go outside the company, because it will never works outside the company.
So for a perfect world the upload filter (but be sure to keep the setting locally and don't override it next download) like the above would be a cleaner solution in most cases in my eyes.

With this I currently don't know any use case for replaceCodeSettings. Because you could always configure something locally and exclude it for uploading. I just need to be sure, that the download does not override the local settings I excluded form uploading.

@shanalikhan
Copy link
Owner

shanalikhan commented Sep 4, 2018

Guys, what're your views about #555.
Feel free to post it there compared with this feature.

I might be closing one of these issues. Functionality is the same but the way is different.
Before that, i want good discussion.

This thread also linked with #292

@shanalikhan shanalikhan modified the milestones: Backlog, v3.2 Sep 4, 2018
@borekb
Copy link
Author

borekb commented Sep 4, 2018

I like it. It would allow me to manage everything from settings.json (I always forget where Sync Settings keep its custom .json config file 😅).

@shanalikhan
Copy link
Owner

Duplicate of #555

@shanalikhan shanalikhan marked this as a duplicate of #555 Sep 4, 2018
@shanalikhan
Copy link
Owner

Locking this thread. Let's keep a track over there.

Repository owner locked and limited conversation to collaborators Sep 4, 2018
@shanalikhan shanalikhan removed this from the v3.2 milestone Sep 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants