{"action":"created","issue":{"url":"https://api.github.com/repos/dimitar-asenov/Envision/issues/171","labels_url":"https://api.github.com/repos/dimitar-asenov/Envision/issues/171/labels{/name}","comments_url":"https://api.github.com/repos/dimitar-asenov/Envision/issues/171/comments","events_url":"https://api.github.com/repos/dimitar-asenov/Envision/issues/171/events","html_url":"https://github.com/dimitar-asenov/Envision/pull/171","id":111560293,"number":171,"title":"Manipulating queries","user":{"login":"lukedirtwalker","id":3530769,"avatar_url":"https://avatars.githubusercontent.com/u/3530769?v=3","gravatar_id":"","url":"https://api.github.com/users/lukedirtwalker","html_url":"https://github.com/lukedirtwalker","followers_url":"https://api.github.com/users/lukedirtwalker/followers","following_url":"https://api.github.com/users/lukedirtwalker/following{/other_user}","gists_url":"https://api.github.com/users/lukedirtwalker/gists{/gist_id}","starred_url":"https://api.github.com/users/lukedirtwalker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lukedirtwalker/subscriptions","organizations_url":"https://api.github.com/users/lukedirtwalker/orgs","repos_url":"https://api.github.com/users/lukedirtwalker/repos","events_url":"https://api.github.com/users/lukedirtwalker/events{/privacy}","received_events_url":"https://api.github.com/users/lukedirtwalker/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-10-15T07:30:23Z","updated_at":"2015-10-16T11:31:03Z","closed_at":"2015-10-16T11:31:03Z","pull_request":{"url":"https://api.github.com/repos/dimitar-asenov/Envision/pulls/171","html_url":"https://github.com/dimitar-asenov/Envision/pull/171","diff_url":"https://github.com/dimitar-asenov/Envision/pull/171.diff","patch_url":"https://github.com/dimitar-asenov/Envision/pull/171.patch"},"body":"This is the current suggestion for how a modifying query could look like. It should serve as a base for discussion.\r\n\r\nWhen considering more cases a more general format for the Tuple given to the modify method could make sense:\r\n* if to while:<br>\r\n  (modify, \\<ifNode\\>), (replaceWith, while), (keep, condition), (as, condition), (keep, thenBranch), (as, body)<br>\r\nhowever this format is not allowed since a key can only appear once, maybe a more condensed format could work:<br>\r\n(modify, \\<ifNode\\>), (replaceWith, while), (keepCondition, condition), (keepThenBranch, body)\r\n\r\n* swap if else:<br>\r\n(modify, \\<ifNode\\>), (swapThenBranch, elseBranch)\r\n\r\n* According to this new format the replace casts would be written as follows\r\n(modify, \\<castNode\\>), (replaceWith, DCast<...>()), (keepExpr, arguments)\r\n\r\n* Extract method:<br>\r\n(modify, \\<nodes to extract\\>), (extractAsMethod, methodName)\n<a href='#crh-start'></a><a href='#crh-data-%7B%22processed%22%3A%20%5B%22https%3A//github.com/dimitar-asenov/Envision/pull/171%23issuecomment-148335625%22%2C%20%22https%3A//github.com/dimitar-asenov/Envision/pull/171%23issuecomment-148673711%22%5D%2C%20%22comments%22%3A%20%7B%22General%20Comment%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/dimitar-asenov/Envision/pull/171%23issuecomment-148335625%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Hmm%2C%20I%20don%27t%20like%20specializing%20the%20tuples%20so%20much.%20In%20particular%20we%20shouldn%27t%20have%20things%20like%20keepCondition/keepThenBranch%20and%20especially%20no%20things%20like%20extractAsMethod%2C%20since%20this%20is%20too%20high-level.%20We%20want%20to%20empower%20developers%20to%20write%20something%20like%20extractAsMethod%2C%20not%20do%20it%20for%20them%20like%20all%20other%20tools%20out%20there.%5Cr%5Cn%5Cr%5CnHow%20does%20the%20replaceWith%20work%3F%20Does%20it%20take%20a%20node%20or%20a%20string%20or%20what%3F%5Cr%5Cn%5Cr%5CnAnd%20how%20does%20the%20last%20modify%20work%20%28with%20%3Cnodes%20to%20extract%3E%29.%20Is%20that%20a%20list%3F%20I%20thought%20modify%20takes%20a%20single%20ast%20node.%5Cr%5Cn%5Cr%5CnNow%20that%20I%20think%20about%20it%2C%20I%27m%20not%20sure%20that%20using%20tuples%20as%20modifications%20command%20is%20the%20best%20choice.%20Perhaps%20tuples%20can%20be%20used%20to%20inform%20a%20python%20script%20which%20does%20the%20modifications%2C%20using%20some%20other%2C%20more%20intuitive%20API.%22%2C%20%22created_at%22%3A%20%222015-10-15T09%3A41%3A21Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1152976%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/dimitar-asenov%22%7D%7D%2C%20%7B%22body%22%3A%20%22Do%20we%20need%20to%20keep%20this%20pull%20request%20alive%2C%20will%20you%20update%20this%20one%20or%20create%20a%20new%20one%20later%3F%22%2C%20%22created_at%22%3A%20%222015-10-16T10%3A05%3A17Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1152976%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/dimitar-asenov%22%7D%7D%5D%2C%20%22title%22%3A%20%22General%20Comment%22%7D%7D%7D'></a>\n<a href='https://www.codereviewhub.com/'><img src='http://www.codereviewhub.com/site/github-bar.png' height=40></a>\n- [ ] <a href='#crh-comment-General Comment'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/dimitar-asenov/Envision/pull/171#issuecomment-148335625'>General Comment</a></b>\n- <a href='https://github.com/dimitar-asenov'><img border=0 src='https://avatars.githubusercontent.com/u/1152976?v=3' height=16 width=16'></a> Hmm, I don't like specializing the tuples so much. In particular we shouldn't have things like keepCondition/keepThenBranch and especially no things like extractAsMethod, since this is too high-level. We want to empower developers to write something like extractAsMethod, not do it for them like all other tools out there.\nHow does the replaceWith work? Does it take a node or a string or what?\nAnd how does the last modify work (with <nodes to extract>). Is that a list? I thought modify takes a single ast node.\nNow that I think about it, I'm not sure that using tuples as modifications command is the best choice. Perhaps tuples can be used to inform a python script which does the modifications, using some other, more intuitive API.\n- <a href='https://github.com/dimitar-asenov'><img border=0 src='https://avatars.githubusercontent.com/u/1152976?v=3' height=16 width=16'></a> Do we need to keep this pull request alive, will you update this one or create a new one later?\n\n\n<a href='https://www.codereviewhub.com/dimitar-asenov/Envision/pull/171?mark_as_completed=1'><img src='http://www.codereviewhub.com/site/github-mark-as-completed.png' height=26></a>&nbsp;<a href='https://www.codereviewhub.com/dimitar-asenov/Envision/pull/171?approve=1'><img src='http://www.codereviewhub.com/site/github-approve.png' height=26></a>&nbsp;<a href='https://github.com/dimitar-asenov/Envision/pull/171'><img src='http://www.codereviewhub.com/site/github-refresh.png' height=26></a>\n<a href='#crh-end'></a>"},"comment":{"url":"https://api.github.com/repos/dimitar-asenov/Envision/issues/comments/148690484","html_url":"https://github.com/dimitar-asenov/Envision/pull/171#issuecomment-148690484","issue_url":"https://api.github.com/repos/dimitar-asenov/Envision/issues/171","id":148690484,"user":{"login":"lukedirtwalker","id":3530769,"avatar_url":"https://avatars.githubusercontent.com/u/3530769?v=3","gravatar_id":"","url":"https://api.github.com/users/lukedirtwalker","html_url":"https://github.com/lukedirtwalker","followers_url":"https://api.github.com/users/lukedirtwalker/followers","following_url":"https://api.github.com/users/lukedirtwalker/following{/other_user}","gists_url":"https://api.github.com/users/lukedirtwalker/gists{/gist_id}","starred_url":"https://api.github.com/users/lukedirtwalker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lukedirtwalker/subscriptions","organizations_url":"https://api.github.com/users/lukedirtwalker/orgs","repos_url":"https://api.github.com/users/lukedirtwalker/repos","events_url":"https://api.github.com/users/lukedirtwalker/events{/privacy}","received_events_url":"https://api.github.com/users/lukedirtwalker/received_events","type":"User","site_admin":false},"created_at":"2015-10-16T11:31:03Z","updated_at":"2015-10-16T11:31:03Z","body":"Closing. Will use a new PR for this feature."}}