Skip to content

Conversation

@gerardroche
Copy link
Contributor

For the most part this is backwards compatible, though there are some edge-case bc breakages.

What does it change?

  1. Setting prefix is renamed from jsdocs_* to docblockr.*.
  2. Settings are now stored in User/Preferences.sublime-settings instead of User/Base File.sublime-settings.

The trickiest part is making sure the refactor is backwards compatible. See here for relevant settings migration code.

Is this something that will be accepted to merge?

I'm happy to address any issues to get this change merged.

@gerardroche gerardroche changed the title Fix settings and the way settings are loaded Rename settings and the way settings are loaded Aug 17, 2016
@ErichDonGubler
Copy link

Question: why not just make this use "User/DocBlockr.sublime-settings" instead? I'm using to using separate settings files for each plugin, and I don't see why we would need to use the default Preferences files here.

@gerardroche
Copy link
Contributor Author

gerardroche commented Jun 6, 2017

@ErichDonGubler There are many more problems with using named settings files than using the built-in preferences. I think it's a bad practice to use named settings files. I wrote up an explanation of this on SO: https://stackoverflow.com/a/43698128 . There are also a few other issues I didn't go into e.g. settings from named settings files are not automatically available to keymaps, in fact how would you make settings from a named settings file available to be used in a keymaps file? See the example of keymaps usage in the SO answer.

So, preferences can be used in keymaps file like so:

{ 
    "keys": [",", "a"], 
    "command": "phpunit_test_suite", 
    "context": [{ "key": "setting.phpunit.keymaps" } ] 
}

How would you do that from settings in a named settings file? There are lots of little issues like this that are solved out-of-the-box by using built-in preferences.

@gerardroche gerardroche deleted the refactor-settings branch January 22, 2020 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants