{"action":"created","comment":{"url":"https://api.github.com/repos/watir/watir/pulls/comments/184913423","pull_request_review_id":116199289,"id":184913423,"diff_hunk":"@@ -70,39 +71,58 @@ def using_selenium(filter = :first)\n \n         def using_watir(filter = :first)\n           selector = selector_builder.normalized_selector\n-          visible = selector.delete(:visible)\n-          visible_text = selector.delete(:visible_text)\n           tag_name = selector[:tag_name].is_a?(::Symbol) ? selector[:tag_name].to_s : selector[:tag_name]\n           validation_required = (selector.key?(:css) || selector.key?(:xpath)) && tag_name\n \n           if selector.key?(:index) && filter == :all\n             raise ArgumentError, \"can't locate all elements by :index\"\n           end\n-          idx = selector.delete(:index) unless selector[:adjacent]\n+\n+          filter_selector = delete_filters_from(selector)\n+          filter_selector[:tag_name] = tag_name if validation_required\n+\n+          query_scope = ensure_scope_context\n+\n+          if filter_selector.key?(:label) && selector_builder.should_use_label_element?\n+            label = label_from_text(filter_selector.delete(:label)) || return","path":"lib/watir/locators/element/locator.rb","position":30,"original_position":30,"commit_id":"7af5a6112bed048f2a8bfa8fa7d9a0686d7c049e","original_commit_id":"7af5a6112bed048f2a8bfa8fa7d9a0686d7c049e","user":{"login":"titusfortner","id":777185,"avatar_url":"https://avatars3.githubusercontent.com/u/777185?v=4","gravatar_id":"","url":"https://api.github.com/users/titusfortner","html_url":"https://github.com/titusfortner","followers_url":"https://api.github.com/users/titusfortner/followers","following_url":"https://api.github.com/users/titusfortner/following{/other_user}","gists_url":"https://api.github.com/users/titusfortner/gists{/gist_id}","starred_url":"https://api.github.com/users/titusfortner/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/titusfortner/subscriptions","organizations_url":"https://api.github.com/users/titusfortner/orgs","repos_url":"https://api.github.com/users/titusfortner/repos","events_url":"https://api.github.com/users/titusfortner/events{/privacy}","received_events_url":"https://api.github.com/users/titusfortner/received_events","type":"User","site_admin":false},"body":"How do you see the responsibilities needing to be split? The only thing that seems obvious to me to figure out how to push it into the SelectorBuilder class is the predicate generation. Everything else is some kind of filtering or validation.","created_at":"2018-04-30T05:32:40Z","updated_at":"2018-04-30T05:32:40Z","html_url":"https://github.com/watir/watir/pull/720#discussion_r184913423","pull_request_url":"https://api.github.com/repos/watir/watir/pulls/720","author_association":"OWNER","_links":{"self":{"href":"https://api.github.com/repos/watir/watir/pulls/comments/184913423"},"html":{"href":"https://github.com/watir/watir/pull/720#discussion_r184913423"},"pull_request":{"href":"https://api.github.com/repos/watir/watir/pulls/720"}},"in_reply_to_id":184904382},"pull_request":{"url":"https://api.github.com/repos/watir/watir/pulls/720","id":184856612,"html_url":"https://github.com/watir/watir/pull/720","diff_url":"https://github.com/watir/watir/pull/720.diff","patch_url":"https://github.com/watir/watir/pull/720.patch","issue_url":"https://api.github.com/repos/watir/watir/issues/720","number":720,"state":"open","locked":false,"title":"Simplify element locator","user":{"login":"jkotests","id":2652587,"avatar_url":"https://avatars1.githubusercontent.com/u/2652587?v=4","gravatar_id":"","url":"https://api.github.com/users/jkotests","html_url":"https://github.com/jkotests","followers_url":"https://api.github.com/users/jkotests/followers","following_url":"https://api.github.com/users/jkotests/following{/other_user}","gists_url":"https://api.github.com/users/jkotests/gists{/gist_id}","starred_url":"https://api.github.com/users/jkotests/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jkotests/subscriptions","organizations_url":"https://api.github.com/users/jkotests/orgs","repos_url":"https://api.github.com/users/jkotests/repos","events_url":"https://api.github.com/users/jkotests/events{/privacy}","received_events_url":"https://api.github.com/users/jkotests/received_events","type":"User","site_admin":false},"body":"@titusfortner, these are the changes I have for simplifying the locator code.\r\n\r\nThe idea is:\r\n\r\n1. Separate locators that can be included in the XPath from those that require filtering (inspecting the element).\r\n2. Build the XPath\r\n3. Apply filters\r\n\r\nTo avoid changing too much, the changes are isolated to the Element::Locator class. At some point, some of the logic could be moved to the selector builder (eg adding of Regexps predicates). \r\n\r\nSome random notes:\r\n* For the specs, the get_element_tag_name wire calls dropped by 76% (from 11,656 to 2,772). \r\n* When locating an element by index and filtering is required, lazy evaluation is used to avoid inspecting elements that are not needed. This should help address the [performance issue raised on Watir General](https://groups.google.com/d/msg/watir-general/vKqwbPyxgTc/JjOs4MuXCAAJ).\r\n","created_at":"2018-04-29T21:43:39Z","updated_at":"2018-04-30T05:32:40Z","closed_at":null,"merged_at":null,"merge_commit_sha":"2e0cbf011a15a2789817eab65fd58713c3f3b067","assignee":null,"assignees":[],"requested_reviewers":[{"login":"p0deje","id":665846,"avatar_url":"https://avatars3.githubusercontent.com/u/665846?v=4","gravatar_id":"","url":"https://api.github.com/users/p0deje","html_url":"https://github.com/p0deje","followers_url":"https://api.github.com/users/p0deje/followers","following_url":"https://api.github.com/users/p0deje/following{/other_user}","gists_url":"https://api.github.com/users/p0deje/gists{/gist_id}","starred_url":"https://api.github.com/users/p0deje/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/p0deje/subscriptions","organizations_url":"https://api.github.com/users/p0deje/orgs","repos_url":"https://api.github.com/users/p0deje/repos","events_url":"https://api.github.com/users/p0deje/events{/privacy}","received_events_url":"https://api.github.com/users/p0deje/received_events","type":"User","site_admin":false}],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/watir/watir/pulls/720/commits","review_comments_url":"https://api.github.com/repos/watir/watir/pulls/720/comments","review_comment_url":"https://api.github.com/repos/watir/watir/pulls/comments{/number}","comments_url":"https://api.github.com/repos/watir/watir/issues/720/comments","statuses_url":"https://api.github.com/repos/watir/watir/statuses/7af5a6112bed048f2a8bfa8fa7d9a0686d7c049e","head":{"label":"jkotests:simplify_locator","ref":"simplify_locator","sha":"7af5a6112bed048f2a8bfa8fa7d9a0686d7c049e","user":{"login":"jkotests","id":2652587,"avatar_url":"https://avatars1.githubusercontent.com/u/2652587?v=4","gravatar_id":"","url":"https://api.github.com/users/jkotests","html_url":"https://github.com/jkotests","followers_url":"https://api.github.com/users/jkotests/followers","following_url":"https://api.github.com/users/jkotests/following{/other_user}","gists_url":"https://api.github.com/users/jkotests/gists{/gist_id}","starred_url":"https://api.github.com/users/jkotests/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jkotests/subscriptions","organizations_url":"https://api.github.com/users/jkotests/orgs","repos_url":"https://api.github.com/users/jkotests/repos","events_url":"https://api.github.com/users/jkotests/events{/privacy}","received_events_url":"https://api.github.com/users/jkotests/received_events","type":"User","site_admin":false},"repo":{"id":14608401,"name":"watir","full_name":"jkotests/watir","owner":{"login":"jkotests","id":2652587,"avatar_url":"https://avatars1.githubusercontent.com/u/2652587?v=4","gravatar_id":"","url":"https://api.github.com/users/jkotests","html_url":"https://github.com/jkotests","followers_url":"https://api.github.com/users/jkotests/followers","following_url":"https://api.github.com/users/jkotests/following{/other_user}","gists_url":"https://api.github.com/users/jkotests/gists{/gist_id}","starred_url":"https://api.github.com/users/jkotests/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jkotests/subscriptions","organizations_url":"https://api.github.com/users/jkotests/orgs","repos_url":"https://api.github.com/users/jkotests/repos","events_url":"https://api.github.com/users/jkotests/events{/privacy}","received_events_url":"https://api.github.com/users/jkotests/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/jkotests/watir","description":"Watir implementation on WebDriver","fork":true,"url":"https://api.github.com/repos/jkotests/watir","forks_url":"https://api.github.com/repos/jkotests/watir/forks","keys_url":"https://api.github.com/repos/jkotests/watir/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jkotests/watir/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jkotests/watir/teams","hooks_url":"https://api.github.com/repos/jkotests/watir/hooks","issue_events_url":"https://api.github.com/repos/jkotests/watir/issues/events{/number}","events_url":"https://api.github.com/repos/jkotests/watir/events","assignees_url":"https://api.github.com/repos/jkotests/watir/assignees{/user}","branches_url":"https://api.github.com/repos/jkotests/watir/branches{/branch}","tags_url":"https://api.github.com/repos/jkotests/watir/tags","blobs_url":"https://api.github.com/repos/jkotests/watir/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jkotests/watir/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jkotests/watir/git/refs{/sha}","trees_url":"https://api.github.com/repos/jkotests/watir/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jkotests/watir/statuses/{sha}","languages_url":"https://api.github.com/repos/jkotests/watir/languages","stargazers_url":"https://api.github.com/repos/jkotests/watir/stargazers","contributors_url":"https://api.github.com/repos/jkotests/watir/contributors","subscribers_url":"https://api.github.com/repos/jkotests/watir/subscribers","subscription_url":"https://api.github.com/repos/jkotests/watir/subscription","commits_url":"https://api.github.com/repos/jkotests/watir/commits{/sha}","git_commits_url":"https://api.github.com/repos/jkotests/watir/git/commits{/sha}","comments_url":"https://api.github.com/repos/jkotests/watir/comments{/number}","issue_comment_url":"https://api.github.com/repos/jkotests/watir/issues/comments{/number}","contents_url":"https://api.github.com/repos/jkotests/watir/contents/{+path}","compare_url":"https://api.github.com/repos/jkotests/watir/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jkotests/watir/merges","archive_url":"https://api.github.com/repos/jkotests/watir/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jkotests/watir/downloads","issues_url":"https://api.github.com/repos/jkotests/watir/issues{/number}","pulls_url":"https://api.github.com/repos/jkotests/watir/pulls{/number}","milestones_url":"https://api.github.com/repos/jkotests/watir/milestones{/number}","notifications_url":"https://api.github.com/repos/jkotests/watir/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jkotests/watir/labels{/name}","releases_url":"https://api.github.com/repos/jkotests/watir/releases{/id}","deployments_url":"https://api.github.com/repos/jkotests/watir/deployments","created_at":"2013-11-22T04:12:18Z","updated_at":"2018-04-29T16:02:19Z","pushed_at":"2018-04-29T20:59:25Z","git_url":"git://github.com/jkotests/watir.git","ssh_url":"git@github.com:jkotests/watir.git","clone_url":"https://github.com/jkotests/watir.git","svn_url":"https://github.com/jkotests/watir","homepage":"http://watirwebdriver.com","size":7440,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"watir:master","ref":"master","sha":"6731cebeda54a14e0a979d45b86bc129a1448ce5","user":{"login":"watir","id":1331009,"avatar_url":"https://avatars2.githubusercontent.com/u/1331009?v=4","gravatar_id":"","url":"https://api.github.com/users/watir","html_url":"https://github.com/watir","followers_url":"https://api.github.com/users/watir/followers","following_url":"https://api.github.com/users/watir/following{/other_user}","gists_url":"https://api.github.com/users/watir/gists{/gist_id}","starred_url":"https://api.github.com/users/watir/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/watir/subscriptions","organizations_url":"https://api.github.com/users/watir/orgs","repos_url":"https://api.github.com/users/watir/repos","events_url":"https://api.github.com/users/watir/events{/privacy}","received_events_url":"https://api.github.com/users/watir/received_events","type":"Organization","site_admin":false},"repo":{"id":366200,"name":"watir","full_name":"watir/watir","owner":{"login":"watir","id":1331009,"avatar_url":"https://avatars2.githubusercontent.com/u/1331009?v=4","gravatar_id":"","url":"https://api.github.com/users/watir","html_url":"https://github.com/watir","followers_url":"https://api.github.com/users/watir/followers","following_url":"https://api.github.com/users/watir/following{/other_user}","gists_url":"https://api.github.com/users/watir/gists{/gist_id}","starred_url":"https://api.github.com/users/watir/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/watir/subscriptions","organizations_url":"https://api.github.com/users/watir/orgs","repos_url":"https://api.github.com/users/watir/repos","events_url":"https://api.github.com/users/watir/events{/privacy}","received_events_url":"https://api.github.com/users/watir/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/watir/watir","description":"Watir Powered By Selenium","fork":false,"url":"https://api.github.com/repos/watir/watir","forks_url":"https://api.github.com/repos/watir/watir/forks","keys_url":"https://api.github.com/repos/watir/watir/keys{/key_id}","collaborators_url":"https://api.github.com/repos/watir/watir/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/watir/watir/teams","hooks_url":"https://api.github.com/repos/watir/watir/hooks","issue_events_url":"https://api.github.com/repos/watir/watir/issues/events{/number}","events_url":"https://api.github.com/repos/watir/watir/events","assignees_url":"https://api.github.com/repos/watir/watir/assignees{/user}","branches_url":"https://api.github.com/repos/watir/watir/branches{/branch}","tags_url":"https://api.github.com/repos/watir/watir/tags","blobs_url":"https://api.github.com/repos/watir/watir/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/watir/watir/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/watir/watir/git/refs{/sha}","trees_url":"https://api.github.com/repos/watir/watir/git/trees{/sha}","statuses_url":"https://api.github.com/repos/watir/watir/statuses/{sha}","languages_url":"https://api.github.com/repos/watir/watir/languages","stargazers_url":"https://api.github.com/repos/watir/watir/stargazers","contributors_url":"https://api.github.com/repos/watir/watir/contributors","subscribers_url":"https://api.github.com/repos/watir/watir/subscribers","subscription_url":"https://api.github.com/repos/watir/watir/subscription","commits_url":"https://api.github.com/repos/watir/watir/commits{/sha}","git_commits_url":"https://api.github.com/repos/watir/watir/git/commits{/sha}","comments_url":"https://api.github.com/repos/watir/watir/comments{/number}","issue_comment_url":"https://api.github.com/repos/watir/watir/issues/comments{/number}","contents_url":"https://api.github.com/repos/watir/watir/contents/{+path}","compare_url":"https://api.github.com/repos/watir/watir/compare/{base}...{head}","merges_url":"https://api.github.com/repos/watir/watir/merges","archive_url":"https://api.github.com/repos/watir/watir/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/watir/watir/downloads","issues_url":"https://api.github.com/repos/watir/watir/issues{/number}","pulls_url":"https://api.github.com/repos/watir/watir/pulls{/number}","milestones_url":"https://api.github.com/repos/watir/watir/milestones{/number}","notifications_url":"https://api.github.com/repos/watir/watir/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/watir/watir/labels{/name}","releases_url":"https://api.github.com/repos/watir/watir/releases{/id}","deployments_url":"https://api.github.com/repos/watir/watir/deployments","created_at":"2009-11-09T11:40:46Z","updated_at":"2018-04-30T04:30:14Z","pushed_at":"2018-04-30T04:30:10Z","git_url":"git://github.com/watir/watir.git","ssh_url":"git@github.com:watir/watir.git","clone_url":"https://github.com/watir/watir.git","svn_url":"https://github.com/watir/watir","homepage":"http://watir.github.io","size":7454,"stargazers_count":1078,"watchers_count":1078,"language":"Ruby","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":188,"mirror_url":null,"archived":false,"open_issues_count":34,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":188,"open_issues":34,"watchers":1078,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/watir/watir/pulls/720"},"html":{"href":"https://github.com/watir/watir/pull/720"},"issue":{"href":"https://api.github.com/repos/watir/watir/issues/720"},"comments":{"href":"https://api.github.com/repos/watir/watir/issues/720/comments"},"review_comments":{"href":"https://api.github.com/repos/watir/watir/pulls/720/comments"},"review_comment":{"href":"https://api.github.com/repos/watir/watir/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/watir/watir/pulls/720/commits"},"statuses":{"href":"https://api.github.com/repos/watir/watir/statuses/7af5a6112bed048f2a8bfa8fa7d9a0686d7c049e"}},"author_association":"OWNER"}}
{"actor":{"display_login":"titusfortner"}}