{"action":"created","issue":{"url":"https://api.github.com/repos/lxde/pcmanfm-qt/issues/391","repository_url":"https://api.github.com/repos/lxde/pcmanfm-qt","labels_url":"https://api.github.com/repos/lxde/pcmanfm-qt/issues/391/labels{/name}","comments_url":"https://api.github.com/repos/lxde/pcmanfm-qt/issues/391/comments","events_url":"https://api.github.com/repos/lxde/pcmanfm-qt/issues/391/events","html_url":"https://github.com/lxde/pcmanfm-qt/pull/391","id":172981630,"number":391,"title":"Per folder settings: a cleaner and simplified implemention.","user":{"login":"PCMan","id":46543,"avatar_url":"https://avatars.githubusercontent.com/u/46543?v=3","gravatar_id":"","url":"https://api.github.com/users/PCMan","html_url":"https://github.com/PCMan","followers_url":"https://api.github.com/users/PCMan/followers","following_url":"https://api.github.com/users/PCMan/following{/other_user}","gists_url":"https://api.github.com/users/PCMan/gists{/gist_id}","starred_url":"https://api.github.com/users/PCMan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PCMan/subscriptions","organizations_url":"https://api.github.com/users/PCMan/orgs","repos_url":"https://api.github.com/users/PCMan/repos","events_url":"https://api.github.com/users/PCMan/events{/privacy}","received_events_url":"https://api.github.com/users/PCMan/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"PCMan","id":46543,"avatar_url":"https://avatars.githubusercontent.com/u/46543?v=3","gravatar_id":"","url":"https://api.github.com/users/PCMan","html_url":"https://github.com/PCMan","followers_url":"https://api.github.com/users/PCMan/followers","following_url":"https://api.github.com/users/PCMan/following{/other_user}","gists_url":"https://api.github.com/users/PCMan/gists{/gist_id}","starred_url":"https://api.github.com/users/PCMan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PCMan/subscriptions","organizations_url":"https://api.github.com/users/PCMan/orgs","repos_url":"https://api.github.com/users/PCMan/repos","events_url":"https://api.github.com/users/PCMan/events{/privacy}","received_events_url":"https://api.github.com/users/PCMan/received_events","type":"User","site_admin":false},"assignees":[{"login":"PCMan","id":46543,"avatar_url":"https://avatars.githubusercontent.com/u/46543?v=3","gravatar_id":"","url":"https://api.github.com/users/PCMan","html_url":"https://github.com/PCMan","followers_url":"https://api.github.com/users/PCMan/followers","following_url":"https://api.github.com/users/PCMan/following{/other_user}","gists_url":"https://api.github.com/users/PCMan/gists{/gist_id}","starred_url":"https://api.github.com/users/PCMan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PCMan/subscriptions","organizations_url":"https://api.github.com/users/PCMan/orgs","repos_url":"https://api.github.com/users/PCMan/repos","events_url":"https://api.github.com/users/PCMan/events{/privacy}","received_events_url":"https://api.github.com/users/PCMan/received_events","type":"User","site_admin":false},{"login":"tsujan","id":981076,"avatar_url":"https://avatars.githubusercontent.com/u/981076?v=3","gravatar_id":"","url":"https://api.github.com/users/tsujan","html_url":"https://github.com/tsujan","followers_url":"https://api.github.com/users/tsujan/followers","following_url":"https://api.github.com/users/tsujan/following{/other_user}","gists_url":"https://api.github.com/users/tsujan/gists{/gist_id}","starred_url":"https://api.github.com/users/tsujan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tsujan/subscriptions","organizations_url":"https://api.github.com/users/tsujan/orgs","repos_url":"https://api.github.com/users/tsujan/repos","events_url":"https://api.github.com/users/tsujan/events{/privacy}","received_events_url":"https://api.github.com/users/tsujan/received_events","type":"User","site_admin":false}],"milestone":null,"comments":1,"created_at":"2016-08-24T15:21:12Z","updated_at":"2016-08-24T15:52:15Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/lxde/pcmanfm-qt/pulls/391","html_url":"https://github.com/lxde/pcmanfm-qt/pull/391","diff_url":"https://github.com/lxde/pcmanfm-qt/pull/391.diff","patch_url":"https://github.com/lxde/pcmanfm-qt/pull/391.patch"},"body":"This new implementation supersedes the old PR #343 with the following changes.\r\n\r\n1. Encapsulation with PCManFM::FolderSettings class and move most of the handling into PCManFM::TabPage rather than adding duplicated pieces of code in MainWindow.\r\n2. Remove the code in onAboutToQuit since it does not work. For example, folders on removable devices  or remote filesystems might have their folder temporarily unavailable, but their per-folder settings should not be deleted.\r\n3. Do not distinguish between per-folder sort settings and per-folder view settings. Separating them is too fine-grained and makes the code complicated. The gtk+ version of pcmanfm uses only one option to control them and this works well enough.\r\n4. Re-use Fm::FolderConfig internally to store the settings.\r\n5. Remove tear-down menus which is an out-dated UI style inconsistent with the rest parts of the DE.\r\n6. Do not change the default view mode when you change the view mode of the current tab. This is a \"feature\", not a bug. The default view mode is configured in the preferences dialog.\r\n\r\n@tsujan I merged your code last week, but after some more code review I found some issues listed above. So later I decided to do some refactor and rewrites."},"comment":{"url":"https://api.github.com/repos/lxde/pcmanfm-qt/issues/comments/242115734","html_url":"https://github.com/lxde/pcmanfm-qt/pull/391#issuecomment-242115734","issue_url":"https://api.github.com/repos/lxde/pcmanfm-qt/issues/391","id":242115734,"user":{"login":"tsujan","id":981076,"avatar_url":"https://avatars.githubusercontent.com/u/981076?v=3","gravatar_id":"","url":"https://api.github.com/users/tsujan","html_url":"https://github.com/tsujan","followers_url":"https://api.github.com/users/tsujan/followers","following_url":"https://api.github.com/users/tsujan/following{/other_user}","gists_url":"https://api.github.com/users/tsujan/gists{/gist_id}","starred_url":"https://api.github.com/users/tsujan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tsujan/subscriptions","organizations_url":"https://api.github.com/users/tsujan/orgs","repos_url":"https://api.github.com/users/tsujan/repos","events_url":"https://api.github.com/users/tsujan/events{/privacy}","received_events_url":"https://api.github.com/users/tsujan/received_events","type":"User","site_admin":false},"created_at":"2016-08-24T15:52:14Z","updated_at":"2016-08-24T15:52:14Z","body":"@PCMan\r\nI'll study and test the code as soon as I have time.\r\n\r\nHowever:\r\n\r\n> Remove the code in onAboutToQuit since it does not work\r\n\r\nWithout it, a lot of unused entries may be left after a while, and that could affect performance. A config file that grows indefinitely is not a good idea, IMHO. On the other hand, users have no urgent need to customize folders related to removable devices or remote filesystems, exactly because they're temporary.\r\n\r\n> Remove tear-down menus which is an out-dated UI style inconsistent with the rest parts of the DE.\r\n\r\nI use my previous PR since I made it and have seen the need for that tear-off. Without it, the user should open the menu twice, which would make customization painful - unless you've done it differently (I haven't tried your code yet).\r\n\r\n> Do not change the default view mode when you change the view mode of the current tab. This is a \"feature\", not a bug.\r\n\r\nI'm afraid it could be confusing to the user when he/she changes the view mode and sees that nothing has changed with other folders -- and not even with same folder later (it was confusing to me). Being confusing, it can't be seen as a feature, IMO."}}
{"actor":{"display_login":"tsujan"}}