{"action":"created","issue":{"url":"https://api.github.com/repos/mixxxdj/mixxx/issues/828","labels_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/828/labels{/name}","comments_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/828/comments","events_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/828/events","html_url":"https://github.com/mixxxdj/mixxx/pull/828","id":123925038,"number":828,"title":"Qsettings","user":{"login":"kain88-de","id":4302678,"avatar_url":"https://avatars.githubusercontent.com/u/4302678?v=3","gravatar_id":"","url":"https://api.github.com/users/kain88-de","html_url":"https://github.com/kain88-de","followers_url":"https://api.github.com/users/kain88-de/followers","following_url":"https://api.github.com/users/kain88-de/following{/other_user}","gists_url":"https://api.github.com/users/kain88-de/gists{/gist_id}","starred_url":"https://api.github.com/users/kain88-de/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kain88-de/subscriptions","organizations_url":"https://api.github.com/users/kain88-de/orgs","repos_url":"https://api.github.com/users/kain88-de/repos","events_url":"https://api.github.com/users/kain88-de/events{/privacy}","received_events_url":"https://api.github.com/users/kain88-de/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2015-12-26T11:06:48Z","updated_at":"2016-01-01T13:13:26Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/828","html_url":"https://github.com/mixxxdj/mixxx/pull/828","diff_url":"https://github.com/mixxxdj/mixxx/pull/828.diff","patch_url":"https://github.com/mixxxdj/mixxx/pull/828.patch"},"body":"This adds read and write functions for Qt QSettings object and a small wrapper class to use QSettings with mixxx.\r\n\r\nI haven't touched any of the config options so far. This should show as a proof-of-concept how we can switch to QSettings and get rid of the `m_pConfig` pointer that we pass around everywhere.  If we want to switch I would suggest merging this PR and then creating a PR for each config-option that has been ported to the new interface. To keep track of the porting progress the QSettings object currently saves it's progress in a file called `Mixxx.cfg` instead of `mixxx.cfg` (notice the capital M).\r\n\r\nOnce we have gotten every access to a config option switch the `mixxx.cfg` file should be empty when we run mixxx with an empty settings-folder (do an `rm -r ~/.mixxx` before starting mixxx). \r\n\r\nA nice side effect of using QSettings is that all options in the settings-file are sorted by group. So every settings associated with the library for example will be at exactly the same place in the file and not spread across it."},"comment":{"url":"https://api.github.com/repos/mixxxdj/mixxx/issues/comments/168305991","html_url":"https://github.com/mixxxdj/mixxx/pull/828#issuecomment-168305991","issue_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/828","id":168305991,"user":{"login":"kain88-de","id":4302678,"avatar_url":"https://avatars.githubusercontent.com/u/4302678?v=3","gravatar_id":"","url":"https://api.github.com/users/kain88-de","html_url":"https://github.com/kain88-de","followers_url":"https://api.github.com/users/kain88-de/followers","following_url":"https://api.github.com/users/kain88-de/following{/other_user}","gists_url":"https://api.github.com/users/kain88-de/gists{/gist_id}","starred_url":"https://api.github.com/users/kain88-de/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kain88-de/subscriptions","organizations_url":"https://api.github.com/users/kain88-de/orgs","repos_url":"https://api.github.com/users/kain88-de/repos","events_url":"https://api.github.com/users/kain88-de/events{/privacy}","received_events_url":"https://api.github.com/users/kain88-de/received_events","type":"User","site_admin":false},"created_at":"2016-01-01T13:13:26Z","updated_at":"2016-01-01T13:13:26Z","body":"> From a software engineering perspective, this allows shortcuts that are \"easier\" at the time you bang out the code but more complicated since it relies on global state. With a little more effort you can arrive at a design that is explicit about its communication needs rather than shortcutting through the settings system. A lot of Mixxx code today already relies on user config as shared mutable state -- I'd like to see us get rid of it rather than make it easier. This makes code harder to maintain and harder to write tests for. This is the same argument I have against singletons.\r\n\r\nWell these are two issues here.\r\n\r\n* For me user configuration *is* a global state by definition. There can't be two different user-configurations in a running instance of mixxx. I also don't have that much of a problem with it because most likely we write to it rarely and from specialized settings dialogs and read often from it in different parts of mixxx. And reading values from the configuration should be easy. For me that should be possible from anywhere inside of mixxx without the need of passing objects along to the target class.\r\nThough I think it should be possible to design a system where only a few special classes can write to the user-config but all can read it.\r\n\r\n* Abusing the current configuration system as a message passing system is not OK. I agree there with you. But I haven't looked into mixxx code for a while so I'm not sure where we are doing that.\r\n\r\nI personally don't think we can get around the user-config being a shared state. There sure might be \r\nways to make it non mutable but I can't think of any right now. If you know of some publicly available examples it would be nice if you link them.\r\n\r\n> For a \"future\" settings format, I would like to move away from a key-value store and towards a structured representation of the configuration (a protocol buffer). This will reduce bugs since the compiler can statically verify that we area accessing a valid preference and make it easier to manage version upgrades and deprecation since we can bundle groups of associated properties into submessages and then unit test the conversion from one submessage type (the deprecated version) to another (the replacement).\r\n\r\nI would still like that the configuration file is human readable. As this made it easier in the past to \r\nhelp some users playing with the configuration to stop mixxx from crashing (mostly related to the waveforms). Defining all configuration options in a separate compilation file and having the compiler check that only those are accessed is a pretty good idea though. If that also includes type checking of the value then it's even better."}}