{"action":"created","comment":{"url":"https://api.github.com/repos/yast/yast-snapper/pulls/comments/65348904","id":65348904,"diff_hunk":"@@ -47,198 +44,409 @@ def initialize_snapper_dialogs(include_target)\n       Yast.include include_target, \"snapper/helps.rb\"\n     end\n \n-\n-    def timestring(t)\n-      return t.strftime(\"%F %T\")\n-    end\n-\n-\n-    def ReallyAbort\n-      Popup.ReallyAbort(true)\n-    end\n-\n-\n-    # Read settings dialog\n-    # @return `abort if aborted and `next otherwise\n-    def ReadDialog\n-      return :abort if !Confirm.MustBeRoot\n-\n-      Wizard.RestoreHelp(Ops.get_string(@HELPS, \"read\", \"\"))\n-      ret = Snapper.Init()\n-      ret ? :next : :abort\n-    end\n-\n-\n     # transform userdata from widget to map\n-    def get_userdata(id)\n-      return Snapper.string_to_userdata(UI.QueryWidget(Id(id), :Value))\n+    def userdata(id)\n+      Snapper.string_to_userdata(UI.QueryWidget(Id(id), :Value).to_s)\n     end\n \n-\n     # generate list of items for Cleanup combo box\n     def cleanup_items(current)\n-      Builtins.maplist([\"timeline\", \"number\", \"\"]) do |cleanup|\n+      [\"timeline\", \"number\", \"\"].map do |cleanup|\n         Item(Id(cleanup), cleanup, cleanup == current)\n       end\n     end\n \n-\n     # compare editable parts of snapshot maps\n-    def snapshot_modified(orig, new)\n-      orig = deep_copy(orig)\n-      new = deep_copy(new)\n-      ret = false\n-      Builtins.foreach(\n-        Convert.convert(new, :from => \"map\", :to => \"map <string, any>\")\n-      ) { |key, value| ret = ret || Ops.get(orig, key) != value }\n-      ret\n+    def snapshot_modified(orig, args)\n+      args.any? { |k, v| orig.send(k) != v }\n     end\n \n-    \n     # grouped enable condition based on snapshot presence for modification widgets\n-    def enable_snapshot_buttons(condition)\n-      UI.ChangeWidget(Id(:show), :Enabled, condition) \n-      UI.ChangeWidget(Id(:modify), :Enabled, condition)\n-      UI.ChangeWidget(Id(:delete), :Enabled, condition)\n+    def enable_buttons(buttons, condition)\n+      buttons.each do |b|\n+        UI.ChangeWidget(Id(b), :Enabled, condition)\n+      end\n     end\n \n-\n     # Popup for modification of existing snapshot\n     # @return true if new snapshot was created\n     def ModifySnapshotPopup(snapshot)\n-      snapshot = deep_copy(snapshot)\n       modified = false\n-      num = Ops.get_integer(snapshot, \"num\", 0)\n-      pre_num = Ops.get_integer(snapshot, \"pre_num\", num)\n-      type = Ops.get_symbol(snapshot, \"type\", :none)\n \n-      pre_index = Ops.get(Snapper.id2index, pre_num, 0)\n-      pre_snapshot = Ops.get(Snapper.snapshots, pre_index, {})\n+      open_modify_dialog(modify_content(snapshot))\n \n-      snapshot_term = lambda do |prefix, data|\n-        data = deep_copy(data)\n-        HBox(\n-          HSpacing(),\n-          Frame(\n-            \"\",\n-            HBox(\n-              HSpacing(0.4),\n-              VBox(\n-                # text entry label\n-                InputField(\n-                  Id(Ops.add(prefix, \"description\")),\n-                  Opt(:hstretch),\n-                  _(\"Description\"),\n-                  Ops.get_string(data, \"description\", \"\")\n-                ),\n-                # text entry label\n-                InputField(\n-                  Id(Ops.add(prefix, \"userdata\")),\n-                  Opt(:hstretch),\n-                  _(\"User data\"),\n-                  Snapper.userdata_to_string(data[\"userdata\"])\n-                ),\n-                Left(\n-                  ComboBox(\n-                    Id(Ops.add(prefix, \"cleanup\")),\n-                    Opt(:editable, :hstretch),\n-                    # combo box label\n-                    _(\"Cleanup algorithm\"),\n-                    cleanup_items(Ops.get_string(data, \"cleanup\", \"\"))\n-                  )\n-                )\n-              ),\n-              HSpacing(0.4)\n-            )\n-          ),\n-          HSpacing()\n-        )\n+      args = {}\n+      pre_args = {}\n+      ret = nil\n+\n+      loop do\n+        ret = UI.UserInput\n+        args = get_modify_args(snapshot)\n+        pre_args = get_modify_args(snapshot.pre) if snapshot.post?\n+        break if ret == :ok || ret == :cancel\n       end\n \n-      if type != :POST\n-        cont = VBox(\n-          # popup label, %{num} is number\n-          Label(_(\"Modify Snapshot %{num}\") % { :num => num }),\n-          snapshot_term.call(\"\", snapshot)\n-        )\n-      else\n-        cont = VBox(\n-          # popup label, %{pre} and %{post} are numbers\n-          Label(_(\"Modify Snapshot %{pre} and %{post}\") % { :pre => pre_num, :post => num }),\n-          # label\n-          Left(Label(_(\"Pre (%{pre})\") % { :pre => pre_num })),\n-          snapshot_term.call(\"pre_\", pre_snapshot),\n-          VSpacing(),\n-          # label\n-          Left(Label(_(\"Post (%{post})\") % { :post => num })),\n-          snapshot_term.call(\"\", snapshot)\n-        )\n+      UI.CloseDialog\n+      if ret == :ok\n+        modified = snapshot.update(args) if snapshot_modified(snapshot, args)\n+\n+        if snapshot.post? && snapshot_modified(snapshot.pre, pre_args)\n+          modified = snapshot.pre.update(pre_args)","path":"src/include/snapper/dialogs.rb","position":167,"original_position":167,"commit_id":"057e4213c28b75174034047e651eefda67449c83","original_commit_id":"057e4213c28b75174034047e651eefda67449c83","user":{"login":"jreidinger","id":478871,"avatar_url":"https://avatars.githubusercontent.com/u/478871?v=3","gravatar_id":"","url":"https://api.github.com/users/jreidinger","html_url":"https://github.com/jreidinger","followers_url":"https://api.github.com/users/jreidinger/followers","following_url":"https://api.github.com/users/jreidinger/following{/other_user}","gists_url":"https://api.github.com/users/jreidinger/gists{/gist_id}","starred_url":"https://api.github.com/users/jreidinger/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jreidinger/subscriptions","organizations_url":"https://api.github.com/users/jreidinger/orgs","repos_url":"https://api.github.com/users/jreidinger/repos","events_url":"https://api.github.com/users/jreidinger/events{/privacy}","received_events_url":"https://api.github.com/users/jreidinger/received_events","type":"User","site_admin":false},"body":"what if this update return false and above one return true? is `false` then valid?","created_at":"2016-06-01T12:19:17Z","updated_at":"2016-06-01T12:19:17Z","html_url":"https://github.com/yast/yast-snapper/pull/40#discussion_r65348904","pull_request_url":"https://api.github.com/repos/yast/yast-snapper/pulls/40","_links":{"self":{"href":"https://api.github.com/repos/yast/yast-snapper/pulls/comments/65348904"},"html":{"href":"https://github.com/yast/yast-snapper/pull/40#discussion_r65348904"},"pull_request":{"href":"https://api.github.com/repos/yast/yast-snapper/pulls/40"}}},"pull_request":{"url":"https://api.github.com/repos/yast/yast-snapper/pulls/40","id":52536708,"html_url":"https://github.com/yast/yast-snapper/pull/40","diff_url":"https://github.com/yast/yast-snapper/pull/40.diff","patch_url":"https://github.com/yast/yast-snapper/pull/40.patch","issue_url":"https://api.github.com/repos/yast/yast-snapper/issues/40","number":40,"state":"open","locked":false,"title":"[WIP] Added snapshot Class with strategy pattern.","user":{"login":"teclator","id":7056681,"avatar_url":"https://avatars.githubusercontent.com/u/7056681?v=3","gravatar_id":"","url":"https://api.github.com/users/teclator","html_url":"https://github.com/teclator","followers_url":"https://api.github.com/users/teclator/followers","following_url":"https://api.github.com/users/teclator/following{/other_user}","gists_url":"https://api.github.com/users/teclator/gists{/gist_id}","starred_url":"https://api.github.com/users/teclator/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/teclator/subscriptions","organizations_url":"https://api.github.com/users/teclator/orgs","repos_url":"https://api.github.com/users/teclator/repos","events_url":"https://api.github.com/users/teclator/events{/privacy}","received_events_url":"https://api.github.com/users/teclator/received_events","type":"User","site_admin":false},"body":"I want to discuss the idea before do more work that could go directly to the trash :P. It's just a preview so please don't be picky about details in the code. \r\n\r\nCleaning up yast-snapper, i missed a more oriented way of use snapshots, so i was planing to add a **Snapshot** class and rewrite **Snapper** to work with Snapshot objects instead of hashes.\r\n\r\nI asked my doubts in the channel and Imobach told me that in **yast-yast2** already exist a similar [class](https://github.com/yast/yast-yast2/blob/master/library/system/src/lib/yast2/fs_snapshot.rb). This class is currently used for installation proposes and when DBus is still not available so Ancorgs suggested the use of this class but rewriting it to use the **[Strategy pattern](https://en.wikipedia.org/wiki/Strategy_pattern)**, so we could have to strategies one for **CLI** and other for **DBus**\r\n\r\nSo what i really do was write a class that could be merged in **FsSnapshot** that use by default the **SnapshotDBus** strategy.\r\n\r\nSo the plan would be remove **SnapperDbus** and rewrite **Snapper** to maintain the actual published methods but calling FsSnapshot directly but being only responsable of the dialog logic.\r\n","created_at":"2015-12-03T13:55:32Z","updated_at":"2016-06-01T12:19:17Z","closed_at":null,"merged_at":null,"merge_commit_sha":"ef2a6f9d9e095c4f9cf35b33e62c1bd966725db0","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/yast/yast-snapper/pulls/40/commits","review_comments_url":"https://api.github.com/repos/yast/yast-snapper/pulls/40/comments","review_comment_url":"https://api.github.com/repos/yast/yast-snapper/pulls/comments{/number}","comments_url":"https://api.github.com/repos/yast/yast-snapper/issues/40/comments","statuses_url":"https://api.github.com/repos/yast/yast-snapper/statuses/057e4213c28b75174034047e651eefda67449c83","head":{"label":"teclator:snapshot_request","ref":"snapshot_request","sha":"057e4213c28b75174034047e651eefda67449c83","user":{"login":"teclator","id":7056681,"avatar_url":"https://avatars.githubusercontent.com/u/7056681?v=3","gravatar_id":"","url":"https://api.github.com/users/teclator","html_url":"https://github.com/teclator","followers_url":"https://api.github.com/users/teclator/followers","following_url":"https://api.github.com/users/teclator/following{/other_user}","gists_url":"https://api.github.com/users/teclator/gists{/gist_id}","starred_url":"https://api.github.com/users/teclator/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/teclator/subscriptions","organizations_url":"https://api.github.com/users/teclator/orgs","repos_url":"https://api.github.com/users/teclator/repos","events_url":"https://api.github.com/users/teclator/events{/privacy}","received_events_url":"https://api.github.com/users/teclator/received_events","type":"User","site_admin":false},"repo":{"id":45900575,"name":"yast-snapper","full_name":"teclator/yast-snapper","owner":{"login":"teclator","id":7056681,"avatar_url":"https://avatars.githubusercontent.com/u/7056681?v=3","gravatar_id":"","url":"https://api.github.com/users/teclator","html_url":"https://github.com/teclator","followers_url":"https://api.github.com/users/teclator/followers","following_url":"https://api.github.com/users/teclator/following{/other_user}","gists_url":"https://api.github.com/users/teclator/gists{/gist_id}","starred_url":"https://api.github.com/users/teclator/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/teclator/subscriptions","organizations_url":"https://api.github.com/users/teclator/orgs","repos_url":"https://api.github.com/users/teclator/repos","events_url":"https://api.github.com/users/teclator/events{/privacy}","received_events_url":"https://api.github.com/users/teclator/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/teclator/yast-snapper","description":"YaST module snapper","fork":true,"url":"https://api.github.com/repos/teclator/yast-snapper","forks_url":"https://api.github.com/repos/teclator/yast-snapper/forks","keys_url":"https://api.github.com/repos/teclator/yast-snapper/keys{/key_id}","collaborators_url":"https://api.github.com/repos/teclator/yast-snapper/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/teclator/yast-snapper/teams","hooks_url":"https://api.github.com/repos/teclator/yast-snapper/hooks","issue_events_url":"https://api.github.com/repos/teclator/yast-snapper/issues/events{/number}","events_url":"https://api.github.com/repos/teclator/yast-snapper/events","assignees_url":"https://api.github.com/repos/teclator/yast-snapper/assignees{/user}","branches_url":"https://api.github.com/repos/teclator/yast-snapper/branches{/branch}","tags_url":"https://api.github.com/repos/teclator/yast-snapper/tags","blobs_url":"https://api.github.com/repos/teclator/yast-snapper/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/teclator/yast-snapper/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/teclator/yast-snapper/git/refs{/sha}","trees_url":"https://api.github.com/repos/teclator/yast-snapper/git/trees{/sha}","statuses_url":"https://api.github.com/repos/teclator/yast-snapper/statuses/{sha}","languages_url":"https://api.github.com/repos/teclator/yast-snapper/languages","stargazers_url":"https://api.github.com/repos/teclator/yast-snapper/stargazers","contributors_url":"https://api.github.com/repos/teclator/yast-snapper/contributors","subscribers_url":"https://api.github.com/repos/teclator/yast-snapper/subscribers","subscription_url":"https://api.github.com/repos/teclator/yast-snapper/subscription","commits_url":"https://api.github.com/repos/teclator/yast-snapper/commits{/sha}","git_commits_url":"https://api.github.com/repos/teclator/yast-snapper/git/commits{/sha}","comments_url":"https://api.github.com/repos/teclator/yast-snapper/comments{/number}","issue_comment_url":"https://api.github.com/repos/teclator/yast-snapper/issues/comments{/number}","contents_url":"https://api.github.com/repos/teclator/yast-snapper/contents/{+path}","compare_url":"https://api.github.com/repos/teclator/yast-snapper/compare/{base}...{head}","merges_url":"https://api.github.com/repos/teclator/yast-snapper/merges","archive_url":"https://api.github.com/repos/teclator/yast-snapper/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/teclator/yast-snapper/downloads","issues_url":"https://api.github.com/repos/teclator/yast-snapper/issues{/number}","pulls_url":"https://api.github.com/repos/teclator/yast-snapper/pulls{/number}","milestones_url":"https://api.github.com/repos/teclator/yast-snapper/milestones{/number}","notifications_url":"https://api.github.com/repos/teclator/yast-snapper/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/teclator/yast-snapper/labels{/name}","releases_url":"https://api.github.com/repos/teclator/yast-snapper/releases{/id}","deployments_url":"https://api.github.com/repos/teclator/yast-snapper/deployments","created_at":"2015-11-10T09:11:48Z","updated_at":"2015-11-10T09:11:49Z","pushed_at":"2016-05-03T08:22:23Z","git_url":"git://github.com/teclator/yast-snapper.git","ssh_url":"git@github.com:teclator/yast-snapper.git","clone_url":"https://github.com/teclator/yast-snapper.git","svn_url":"https://github.com/teclator/yast-snapper","homepage":"http://en.opensuse.org/Portal:YaST","size":291,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"yast:master","ref":"master","sha":"dadf674abb26bef94ae93f065f1e82d495e6b563","user":{"login":"yast","id":909990,"avatar_url":"https://avatars.githubusercontent.com/u/909990?v=3","gravatar_id":"","url":"https://api.github.com/users/yast","html_url":"https://github.com/yast","followers_url":"https://api.github.com/users/yast/followers","following_url":"https://api.github.com/users/yast/following{/other_user}","gists_url":"https://api.github.com/users/yast/gists{/gist_id}","starred_url":"https://api.github.com/users/yast/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yast/subscriptions","organizations_url":"https://api.github.com/users/yast/orgs","repos_url":"https://api.github.com/users/yast/repos","events_url":"https://api.github.com/users/yast/events{/privacy}","received_events_url":"https://api.github.com/users/yast/received_events","type":"Organization","site_admin":false},"repo":{"id":4368216,"name":"yast-snapper","full_name":"yast/yast-snapper","owner":{"login":"yast","id":909990,"avatar_url":"https://avatars.githubusercontent.com/u/909990?v=3","gravatar_id":"","url":"https://api.github.com/users/yast","html_url":"https://github.com/yast","followers_url":"https://api.github.com/users/yast/followers","following_url":"https://api.github.com/users/yast/following{/other_user}","gists_url":"https://api.github.com/users/yast/gists{/gist_id}","starred_url":"https://api.github.com/users/yast/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yast/subscriptions","organizations_url":"https://api.github.com/users/yast/orgs","repos_url":"https://api.github.com/users/yast/repos","events_url":"https://api.github.com/users/yast/events{/privacy}","received_events_url":"https://api.github.com/users/yast/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/yast/yast-snapper","description":"YaST module snapper","fork":false,"url":"https://api.github.com/repos/yast/yast-snapper","forks_url":"https://api.github.com/repos/yast/yast-snapper/forks","keys_url":"https://api.github.com/repos/yast/yast-snapper/keys{/key_id}","collaborators_url":"https://api.github.com/repos/yast/yast-snapper/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/yast/yast-snapper/teams","hooks_url":"https://api.github.com/repos/yast/yast-snapper/hooks","issue_events_url":"https://api.github.com/repos/yast/yast-snapper/issues/events{/number}","events_url":"https://api.github.com/repos/yast/yast-snapper/events","assignees_url":"https://api.github.com/repos/yast/yast-snapper/assignees{/user}","branches_url":"https://api.github.com/repos/yast/yast-snapper/branches{/branch}","tags_url":"https://api.github.com/repos/yast/yast-snapper/tags","blobs_url":"https://api.github.com/repos/yast/yast-snapper/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/yast/yast-snapper/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/yast/yast-snapper/git/refs{/sha}","trees_url":"https://api.github.com/repos/yast/yast-snapper/git/trees{/sha}","statuses_url":"https://api.github.com/repos/yast/yast-snapper/statuses/{sha}","languages_url":"https://api.github.com/repos/yast/yast-snapper/languages","stargazers_url":"https://api.github.com/repos/yast/yast-snapper/stargazers","contributors_url":"https://api.github.com/repos/yast/yast-snapper/contributors","subscribers_url":"https://api.github.com/repos/yast/yast-snapper/subscribers","subscription_url":"https://api.github.com/repos/yast/yast-snapper/subscription","commits_url":"https://api.github.com/repos/yast/yast-snapper/commits{/sha}","git_commits_url":"https://api.github.com/repos/yast/yast-snapper/git/commits{/sha}","comments_url":"https://api.github.com/repos/yast/yast-snapper/comments{/number}","issue_comment_url":"https://api.github.com/repos/yast/yast-snapper/issues/comments{/number}","contents_url":"https://api.github.com/repos/yast/yast-snapper/contents/{+path}","compare_url":"https://api.github.com/repos/yast/yast-snapper/compare/{base}...{head}","merges_url":"https://api.github.com/repos/yast/yast-snapper/merges","archive_url":"https://api.github.com/repos/yast/yast-snapper/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/yast/yast-snapper/downloads","issues_url":"https://api.github.com/repos/yast/yast-snapper/issues{/number}","pulls_url":"https://api.github.com/repos/yast/yast-snapper/pulls{/number}","milestones_url":"https://api.github.com/repos/yast/yast-snapper/milestones{/number}","notifications_url":"https://api.github.com/repos/yast/yast-snapper/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/yast/yast-snapper/labels{/name}","releases_url":"https://api.github.com/repos/yast/yast-snapper/releases{/id}","deployments_url":"https://api.github.com/repos/yast/yast-snapper/deployments","created_at":"2012-05-18T10:26:26Z","updated_at":"2015-04-17T07:49:47Z","pushed_at":"2016-05-03T08:22:23Z","git_url":"git://github.com/yast/yast-snapper.git","ssh_url":"git@github.com:yast/yast-snapper.git","clone_url":"https://github.com/yast/yast-snapper.git","svn_url":"https://github.com/yast/yast-snapper","homepage":"http://en.opensuse.org/Portal:YaST","size":258,"stargazers_count":1,"watchers_count":1,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":5,"mirror_url":null,"open_issues_count":1,"forks":5,"open_issues":1,"watchers":1,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/yast/yast-snapper/pulls/40"},"html":{"href":"https://github.com/yast/yast-snapper/pull/40"},"issue":{"href":"https://api.github.com/repos/yast/yast-snapper/issues/40"},"comments":{"href":"https://api.github.com/repos/yast/yast-snapper/issues/40/comments"},"review_comments":{"href":"https://api.github.com/repos/yast/yast-snapper/pulls/40/comments"},"review_comment":{"href":"https://api.github.com/repos/yast/yast-snapper/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/yast/yast-snapper/pulls/40/commits"},"statuses":{"href":"https://api.github.com/repos/yast/yast-snapper/statuses/057e4213c28b75174034047e651eefda67449c83"}}}}
{"actor":{"display_login":"jreidinger"}}