search

mfuentesg / SyncSettings

Sync Settings - The cross-platform solution to keep Sublime Text configuration synchronized
https://mfuentesg.dev/SyncSettings
MIT License
288 stars 38 forks source link

implement: json to sublime.decode_value/encode_value conversion #151

Closed TerminalFi closed 4 years ago

TerminalFi commented 4 years ago

fixes #144 fixes #119

BLUF Python's json library doesn't allow for trailing comments, or comments 🤷‍♂️ because it follows JSON Spec. Sublime files however often time have these, as sublime allows them. So too accommodate for that, we use sublimes builtin encode_value and decode_value for handling json.

@mfuentesg Not sure how to implement test case. Sorry, I've not learned python 100%. Could use some general guidance.

mfuentesg commented 4 years ago

@TheSecEng you need to create mock for encode_value and decode_value functions. Take a look to this file.

One way to test this change is, is implementing json.loads in sublime.decode_value.

For example here, i am mocking the behavior of sublime.yes_no_cancel_dialog function, try to do the same with sublime.decode_value.

https://github.com/mfuentesg/SyncSettings/blob/a75694803ecee02a463f545ef3754b537f8ed70c/tests/test_sync_version.py#L45-L52

mfuentesg commented 4 years ago

@TheSecEng , please try to run your tests locally, before to push. Thanks in advance for your contribution.

TerminalFi commented 4 years ago

@mfuentesg thanks for some direction, looks like I have that portion working, not sure how to modify the get_local_version tests to pass since they are using sublime functions

mfuentesg commented 4 years ago

@TheSecEng i will check it after my work hours.

mfuentesg commented 4 years ago

i'll try to help you to write tests

TerminalFi commented 4 years ago

I want to add that the following functions were adding in Sublime Text 3019, likely not a problem.

sublime.encode_value sublime.decode_value

TerminalFi commented 4 years ago

@mfuentesg I am sorry I can't help more with writing the tests. Have you been able to look at this more?

mfuentesg commented 4 years ago

Hey @TheSecEng thanks for your time, i will try to take it next week.

dalisoft commented 4 years ago

Who doesn't wait or need currently this feature, do these steps

This feature works only in Unix systems (macOS, Linux, WSL)

  1. Remove current Sync Settings (Disabling doesn't work)
  2. Press Cmd/Ctrl + Shift + P, search Browser Packages and click Enter
  3. Copy directory path and paste go-to directory via cd DIR or Open terminal here at directory via context menu (available for Linux)
  4. Run git clone https://github.com/TheSecEng/SyncSettings.git in Terminal
  5. Run cd SyncSettings && git checkout fix/json-handling in Terminal
  6. Restart Sublime Text
  7. Press Cmd/Ctrl + Shift + P again, but now search Satisfy Dependencies and click Enter
  8. Restart Sublime Text

Done, you should get working solution

Thanks to @mfuentesg - for original best package, thanks billion times @TheSecEng - for fork and fix, thanks billion times and to all contributors of SyncSettings package + Sublime Text teams for fast editor

Regards, Davlik

TerminalFi commented 4 years ago

Who doesn't wait or need currently this feature, do these steps

This feature works only in Unix systems (macOS, Linux, WSL)

  1. Remove current Sync Settings (Disabling doesn't work)
  2. Press Cmd/Ctrl + Shift + P, search Browser Packages and click Enter
  3. Copy directory path and paste go-to directory via cd DIR or Open terminal here at directory via context menu (available for Linux)
  4. Run git clone https://github.com/TheSecEng/SyncSettings.git in Terminal
  5. Run cd SyncSettings && git checkout fix/json-handling in Terminal
  6. Restart Sublime Text
  7. Press Cmd/Ctrl + Shift + P again, but now search Satisfy Dependencies and click Enter
  8. Restart Sublime Text

Done, you should get working solution

Thanks to @mfuentesg - for original best package, thanks billion times @TheSecEng - for fork and fix, thanks billion times and to all contributors of SyncSettings package + Sublime Text teams for fast editor

Regards, Davlik

I highly suggest not doing this. I will not be maintaining my branch and it will be deleted once this is merged.

dalisoft commented 4 years ago

@TheSecEng after this is merged, we will have original package with fixes and these steps does not need. But thanks for warning

mfuentesg commented 4 years ago

@dalisoft @TheSecEng thank you guys, i am very busy with my job, i haven't the bandwidth yet. Give me some time please :D.

Sorry for the inconvenience

mfuentesg commented 4 years ago

@TheSecEng plase find my comments to solve the issue.

mfuentesg commented 4 years ago

@allcontributors please add @TheSecEng for code

allcontributors[bot] commented 4 years ago

@mfuentesg

I've put up a pull request to add @TheSecEng! :tada:

TerminalFi commented 4 years ago

@mfuentesg Thanks, all is good finally.

mfuentesg commented 4 years ago

@allcontributors please add @TheSecEng for code

allcontributors[bot] commented 4 years ago

@mfuentesg

I've put up a pull request to add @TheSecEng! :tada:

dalisoft commented 3 years ago

As #167 is merged, seems, this way is only one to get it working on ST4 on macOS?

mfuentesg commented 3 years ago

@dalisoft those changes are already deployed and tagged, using latest version of ST should has this fixed.

dalisoft commented 3 years ago

@mfuentesg I know this changed already merged, but also reverted via #167, isn't? I love ST and amazing plugins made by community, but it's not working, only way to get it working is same comment as linked above. Thanks a lot for your hard work to make this plugin

dalisoft commented 3 years ago
image

My gist settings is here

mfuentesg commented 3 years ago

@dalisoft please find issue here. https://github.com/mfuentesg/SyncSettings/issues/185

dalisoft commented 3 years ago

@mfuentesg Thanks

mfuentesg commented 3 years ago

@dalisoft https://github.com/mfuentesg/SyncSettings/issues/185 this is already merged, please let me know if it solve your issue 👍🏻

Any other comment, feel free to reopen issue https://github.com/mfuentesg/SyncSettings/issues/185