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

fix: `json.loads` trailing comma #145

Closed TerminalFi closed 4 years ago

TerminalFi commented 4 years ago

Issue When syncing settings, some sublime-settings file contain trailing comma's. The json package doesn't handle trailing comma's very well.

Solution

  1. Removed json library
  2. Implemented sublime.encode_value and sublime.decode_value
mfuentesg commented 4 years ago

Hello, please, can you rebase your branch with the latest changes.

mfuentesg commented 4 years ago

It's very difficult to follow the changes due to all "quotes" changes.

Please, just keep related changes and keep using single quotes for everything.

Thanks for contributing!

TerminalFi commented 4 years ago

Sure, and sorry, that was my LSP correcting the quotes. I’ll rebase and revert quote changes tonight

mfuentesg commented 4 years ago

In general LGTM, but you need to ensure that your changes are working properly.

TerminalFi commented 4 years ago

I'll work on the test case tonight

TerminalFi commented 4 years ago

@mfuentesg I am having trouble creating a test case for this, I can recreate the issue with my files locally every time, however creating a test case isn't appearing as easy.

For the time being, I will revert a few changes until I can get a test case working.

TerminalFi commented 4 years ago

@mfuentesg I have never written a test case. Can you validate this one?

TerminalFi commented 4 years ago

@mfuentesg going to re-write this with a more clean implementation