{"action":"created","comment":{"url":"https://api.github.com/repos/thoughtbot/gitsh/pulls/comments/72962017","id":72962017,"diff_hunk":"@@ -8,7 +8,8 @@ def initialize(env)\n     end\n \n     def initialized?\n-      git_dir && File.exist?(git_dir)\n+      dir = git_dir\n+      dir && File.exist?(dir)","path":"lib/gitsh/git_repository.rb","position":6,"original_position":6,"commit_id":"f1bba1e9238b0aa3d29ff7fdc996a7da5baeb554","original_commit_id":"f1bba1e9238b0aa3d29ff7fdc996a7da5baeb554","user":{"login":"mike-burns","id":4550,"avatar_url":"https://avatars.githubusercontent.com/u/4550?v=3","gravatar_id":"","url":"https://api.github.com/users/mike-burns","html_url":"https://github.com/mike-burns","followers_url":"https://api.github.com/users/mike-burns/followers","following_url":"https://api.github.com/users/mike-burns/following{/other_user}","gists_url":"https://api.github.com/users/mike-burns/gists{/gist_id}","starred_url":"https://api.github.com/users/mike-burns/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mike-burns/subscriptions","organizations_url":"https://api.github.com/users/mike-burns/orgs","repos_url":"https://api.github.com/users/mike-burns/repos","events_url":"https://api.github.com/users/mike-burns/events{/privacy}","received_events_url":"https://api.github.com/users/mike-burns/received_events","type":"User","site_admin":false},"body":"I'm confused in all these suggestions -- not sure what I'm missing -- but what is the `TypeError` that would be raised? Can `git_output` produce `nil`?","created_at":"2016-08-01T11:21:24Z","updated_at":"2016-08-01T11:21:24Z","html_url":"https://github.com/thoughtbot/gitsh/pull/264#discussion_r72962017","pull_request_url":"https://api.github.com/repos/thoughtbot/gitsh/pulls/264","_links":{"self":{"href":"https://api.github.com/repos/thoughtbot/gitsh/pulls/comments/72962017"},"html":{"href":"https://github.com/thoughtbot/gitsh/pull/264#discussion_r72962017"},"pull_request":{"href":"https://api.github.com/repos/thoughtbot/gitsh/pulls/264"}}},"pull_request":{"url":"https://api.github.com/repos/thoughtbot/gitsh/pulls/264","id":79489359,"html_url":"https://github.com/thoughtbot/gitsh/pull/264","diff_url":"https://github.com/thoughtbot/gitsh/pull/264.diff","patch_url":"https://github.com/thoughtbot/gitsh/pull/264.patch","issue_url":"https://api.github.com/repos/thoughtbot/gitsh/issues/264","number":264,"state":"open","locked":false,"title":"Improve prompt generation speed","user":{"login":"calleerlandsson","id":66666,"avatar_url":"https://avatars.githubusercontent.com/u/66666?v=3","gravatar_id":"","url":"https://api.github.com/users/calleerlandsson","html_url":"https://github.com/calleerlandsson","followers_url":"https://api.github.com/users/calleerlandsson/followers","following_url":"https://api.github.com/users/calleerlandsson/following{/other_user}","gists_url":"https://api.github.com/users/calleerlandsson/gists{/gist_id}","starred_url":"https://api.github.com/users/calleerlandsson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/calleerlandsson/subscriptions","organizations_url":"https://api.github.com/users/calleerlandsson/orgs","repos_url":"https://api.github.com/users/calleerlandsson/repos","events_url":"https://api.github.com/users/calleerlandsson/events{/privacy}","received_events_url":"https://api.github.com/users/calleerlandsson/received_events","type":"User","site_admin":false},"body":"Generating and displaying the prompt in gitsh is currently painfully slow on my MacBook Air. This PR is an attempt to improve on this.\r\n\r\nI started to investigate by printing all git commands passed to `Open3.capture3` in `GitRepository#git_output`:\r\n\r\n```\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git symbolic-ref HEAD --short\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git symbolic-ref HEAD --short\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git symbolic-ref HEAD --short\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git status --porcelain\"\r\n\"/usr/bin/env git status --porcelain\"\r\n\"/usr/bin/env git config --get-color gitsh.color.modified yellow\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git status --porcelain\"\r\n\"/usr/bin/env git status --porcelain\"\r\n```\r\n\r\nGenerating the prompt on my system took ~1.1 seconds.\r\n\r\nI observed that most of the calls to external git commands were duplicated.\r\n\r\nI then added `puts` statements in many of the methods in `Prompter` and `PromptColor` in order to find out where the calls were made. I went on to reduce this number by introducing memoization in a way that, to my knowledge, doesn't affect the behavior of the program.\r\n\r\nThe three commits in this PR are the results of this. It brings the generation time down to 0.3 seconds and the following calls to external git commands:\r\n\r\n```\r\n\"/usr/bin/env git rev-parse --git-dir\"\r\n\"/usr/bin/env git status --porcelain\"\r\n\"/usr/bin/env git config --get-color gitsh.color.modified yellow\"\r\n\"/usr/bin/env git symbolic-ref HEAD --short\"\r\n```\r\n\r\nIn doing this I exposed a new `status` method on `GitRepository`. I didn't figure out a good way to test this new method. It is currently implicitly tested through the `has_untracked_files?` and `has_modified_files?` specs. I considered the following options:\r\n\r\n1. Duplicate the existing tests but in the exercise, call `status.has_untracked_files?` instead of `has_untracked_files?`. I don't think the duplicated tests would provide much value.\r\n2. Create a new spec for `GitRepository::Status` passing in example `git status --porcelain` output and replace the existing specs with specs that test that the original methods delegate to the status object. Since the existing specs tests the git repository on an integration level by altering a real git repository and the new specs would be hard to implement this way, this option seems to reduce the value of the test suite.\r\n\r\nI'd love some input on this.\r\n\r\nIn my last commit I also inline the `PromptColor` method within the `Prompter` to make use of the cache inside it. Only having one method and no private helpers, it seemed like the `PromptColor` didn't really pull it's own weight so I think this is a good trade off. I'm also unsure of the value provided by the `PromptColor` spec since it mostly duplicated the conditional logic of the implementation. Therefor I didn't add any similar tests for `Prompter`.","created_at":"2016-07-31T16:28:43Z","updated_at":"2016-08-01T11:21:24Z","closed_at":null,"merged_at":null,"merge_commit_sha":"270375781449c4fa573b46f542184c6a0a430006","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/thoughtbot/gitsh/pulls/264/commits","review_comments_url":"https://api.github.com/repos/thoughtbot/gitsh/pulls/264/comments","review_comment_url":"https://api.github.com/repos/thoughtbot/gitsh/pulls/comments{/number}","comments_url":"https://api.github.com/repos/thoughtbot/gitsh/issues/264/comments","statuses_url":"https://api.github.com/repos/thoughtbot/gitsh/statuses/f1bba1e9238b0aa3d29ff7fdc996a7da5baeb554","head":{"label":"thoughtbot:prompt-speed","ref":"prompt-speed","sha":"f1bba1e9238b0aa3d29ff7fdc996a7da5baeb554","user":{"login":"thoughtbot","id":6183,"avatar_url":"https://avatars.githubusercontent.com/u/6183?v=3","gravatar_id":"","url":"https://api.github.com/users/thoughtbot","html_url":"https://github.com/thoughtbot","followers_url":"https://api.github.com/users/thoughtbot/followers","following_url":"https://api.github.com/users/thoughtbot/following{/other_user}","gists_url":"https://api.github.com/users/thoughtbot/gists{/gist_id}","starred_url":"https://api.github.com/users/thoughtbot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thoughtbot/subscriptions","organizations_url":"https://api.github.com/users/thoughtbot/orgs","repos_url":"https://api.github.com/users/thoughtbot/repos","events_url":"https://api.github.com/users/thoughtbot/events{/privacy}","received_events_url":"https://api.github.com/users/thoughtbot/received_events","type":"Organization","site_admin":false},"repo":{"id":14063942,"name":"gitsh","full_name":"thoughtbot/gitsh","owner":{"login":"thoughtbot","id":6183,"avatar_url":"https://avatars.githubusercontent.com/u/6183?v=3","gravatar_id":"","url":"https://api.github.com/users/thoughtbot","html_url":"https://github.com/thoughtbot","followers_url":"https://api.github.com/users/thoughtbot/followers","following_url":"https://api.github.com/users/thoughtbot/following{/other_user}","gists_url":"https://api.github.com/users/thoughtbot/gists{/gist_id}","starred_url":"https://api.github.com/users/thoughtbot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thoughtbot/subscriptions","organizations_url":"https://api.github.com/users/thoughtbot/orgs","repos_url":"https://api.github.com/users/thoughtbot/repos","events_url":"https://api.github.com/users/thoughtbot/events{/privacy}","received_events_url":"https://api.github.com/users/thoughtbot/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/thoughtbot/gitsh","description":"An interactive shell for git","fork":false,"url":"https://api.github.com/repos/thoughtbot/gitsh","forks_url":"https://api.github.com/repos/thoughtbot/gitsh/forks","keys_url":"https://api.github.com/repos/thoughtbot/gitsh/keys{/key_id}","collaborators_url":"https://api.github.com/repos/thoughtbot/gitsh/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/thoughtbot/gitsh/teams","hooks_url":"https://api.github.com/repos/thoughtbot/gitsh/hooks","issue_events_url":"https://api.github.com/repos/thoughtbot/gitsh/issues/events{/number}","events_url":"https://api.github.com/repos/thoughtbot/gitsh/events","assignees_url":"https://api.github.com/repos/thoughtbot/gitsh/assignees{/user}","branches_url":"https://api.github.com/repos/thoughtbot/gitsh/branches{/branch}","tags_url":"https://api.github.com/repos/thoughtbot/gitsh/tags","blobs_url":"https://api.github.com/repos/thoughtbot/gitsh/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/thoughtbot/gitsh/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/thoughtbot/gitsh/git/refs{/sha}","trees_url":"https://api.github.com/repos/thoughtbot/gitsh/git/trees{/sha}","statuses_url":"https://api.github.com/repos/thoughtbot/gitsh/statuses/{sha}","languages_url":"https://api.github.com/repos/thoughtbot/gitsh/languages","stargazers_url":"https://api.github.com/repos/thoughtbot/gitsh/stargazers","contributors_url":"https://api.github.com/repos/thoughtbot/gitsh/contributors","subscribers_url":"https://api.github.com/repos/thoughtbot/gitsh/subscribers","subscription_url":"https://api.github.com/repos/thoughtbot/gitsh/subscription","commits_url":"https://api.github.com/repos/thoughtbot/gitsh/commits{/sha}","git_commits_url":"https://api.github.com/repos/thoughtbot/gitsh/git/commits{/sha}","comments_url":"https://api.github.com/repos/thoughtbot/gitsh/comments{/number}","issue_comment_url":"https://api.github.com/repos/thoughtbot/gitsh/issues/comments{/number}","contents_url":"https://api.github.com/repos/thoughtbot/gitsh/contents/{+path}","compare_url":"https://api.github.com/repos/thoughtbot/gitsh/compare/{base}...{head}","merges_url":"https://api.github.com/repos/thoughtbot/gitsh/merges","archive_url":"https://api.github.com/repos/thoughtbot/gitsh/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/thoughtbot/gitsh/downloads","issues_url":"https://api.github.com/repos/thoughtbot/gitsh/issues{/number}","pulls_url":"https://api.github.com/repos/thoughtbot/gitsh/pulls{/number}","milestones_url":"https://api.github.com/repos/thoughtbot/gitsh/milestones{/number}","notifications_url":"https://api.github.com/repos/thoughtbot/gitsh/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/thoughtbot/gitsh/labels{/name}","releases_url":"https://api.github.com/repos/thoughtbot/gitsh/releases{/id}","deployments_url":"https://api.github.com/repos/thoughtbot/gitsh/deployments","created_at":"2013-11-02T09:33:38Z","updated_at":"2016-07-30T21:40:47Z","pushed_at":"2016-07-31T16:37:16Z","git_url":"git://github.com/thoughtbot/gitsh.git","ssh_url":"git@github.com:thoughtbot/gitsh.git","clone_url":"https://github.com/thoughtbot/gitsh.git","svn_url":"https://github.com/thoughtbot/gitsh","homepage":"https://thoughtbot.com/tools","size":2036,"stargazers_count":1581,"watchers_count":1581,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":73,"mirror_url":null,"open_issues_count":27,"forks":73,"open_issues":27,"watchers":1581,"default_branch":"master"}},"base":{"label":"thoughtbot:master","ref":"master","sha":"ea9bb5c9b703bcb2dd1f351a2ce8a0be10a65de8","user":{"login":"thoughtbot","id":6183,"avatar_url":"https://avatars.githubusercontent.com/u/6183?v=3","gravatar_id":"","url":"https://api.github.com/users/thoughtbot","html_url":"https://github.com/thoughtbot","followers_url":"https://api.github.com/users/thoughtbot/followers","following_url":"https://api.github.com/users/thoughtbot/following{/other_user}","gists_url":"https://api.github.com/users/thoughtbot/gists{/gist_id}","starred_url":"https://api.github.com/users/thoughtbot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thoughtbot/subscriptions","organizations_url":"https://api.github.com/users/thoughtbot/orgs","repos_url":"https://api.github.com/users/thoughtbot/repos","events_url":"https://api.github.com/users/thoughtbot/events{/privacy}","received_events_url":"https://api.github.com/users/thoughtbot/received_events","type":"Organization","site_admin":false},"repo":{"id":14063942,"name":"gitsh","full_name":"thoughtbot/gitsh","owner":{"login":"thoughtbot","id":6183,"avatar_url":"https://avatars.githubusercontent.com/u/6183?v=3","gravatar_id":"","url":"https://api.github.com/users/thoughtbot","html_url":"https://github.com/thoughtbot","followers_url":"https://api.github.com/users/thoughtbot/followers","following_url":"https://api.github.com/users/thoughtbot/following{/other_user}","gists_url":"https://api.github.com/users/thoughtbot/gists{/gist_id}","starred_url":"https://api.github.com/users/thoughtbot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thoughtbot/subscriptions","organizations_url":"https://api.github.com/users/thoughtbot/orgs","repos_url":"https://api.github.com/users/thoughtbot/repos","events_url":"https://api.github.com/users/thoughtbot/events{/privacy}","received_events_url":"https://api.github.com/users/thoughtbot/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/thoughtbot/gitsh","description":"An interactive shell for git","fork":false,"url":"https://api.github.com/repos/thoughtbot/gitsh","forks_url":"https://api.github.com/repos/thoughtbot/gitsh/forks","keys_url":"https://api.github.com/repos/thoughtbot/gitsh/keys{/key_id}","collaborators_url":"https://api.github.com/repos/thoughtbot/gitsh/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/thoughtbot/gitsh/teams","hooks_url":"https://api.github.com/repos/thoughtbot/gitsh/hooks","issue_events_url":"https://api.github.com/repos/thoughtbot/gitsh/issues/events{/number}","events_url":"https://api.github.com/repos/thoughtbot/gitsh/events","assignees_url":"https://api.github.com/repos/thoughtbot/gitsh/assignees{/user}","branches_url":"https://api.github.com/repos/thoughtbot/gitsh/branches{/branch}","tags_url":"https://api.github.com/repos/thoughtbot/gitsh/tags","blobs_url":"https://api.github.com/repos/thoughtbot/gitsh/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/thoughtbot/gitsh/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/thoughtbot/gitsh/git/refs{/sha}","trees_url":"https://api.github.com/repos/thoughtbot/gitsh/git/trees{/sha}","statuses_url":"https://api.github.com/repos/thoughtbot/gitsh/statuses/{sha}","languages_url":"https://api.github.com/repos/thoughtbot/gitsh/languages","stargazers_url":"https://api.github.com/repos/thoughtbot/gitsh/stargazers","contributors_url":"https://api.github.com/repos/thoughtbot/gitsh/contributors","subscribers_url":"https://api.github.com/repos/thoughtbot/gitsh/subscribers","subscription_url":"https://api.github.com/repos/thoughtbot/gitsh/subscription","commits_url":"https://api.github.com/repos/thoughtbot/gitsh/commits{/sha}","git_commits_url":"https://api.github.com/repos/thoughtbot/gitsh/git/commits{/sha}","comments_url":"https://api.github.com/repos/thoughtbot/gitsh/comments{/number}","issue_comment_url":"https://api.github.com/repos/thoughtbot/gitsh/issues/comments{/number}","contents_url":"https://api.github.com/repos/thoughtbot/gitsh/contents/{+path}","compare_url":"https://api.github.com/repos/thoughtbot/gitsh/compare/{base}...{head}","merges_url":"https://api.github.com/repos/thoughtbot/gitsh/merges","archive_url":"https://api.github.com/repos/thoughtbot/gitsh/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/thoughtbot/gitsh/downloads","issues_url":"https://api.github.com/repos/thoughtbot/gitsh/issues{/number}","pulls_url":"https://api.github.com/repos/thoughtbot/gitsh/pulls{/number}","milestones_url":"https://api.github.com/repos/thoughtbot/gitsh/milestones{/number}","notifications_url":"https://api.github.com/repos/thoughtbot/gitsh/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/thoughtbot/gitsh/labels{/name}","releases_url":"https://api.github.com/repos/thoughtbot/gitsh/releases{/id}","deployments_url":"https://api.github.com/repos/thoughtbot/gitsh/deployments","created_at":"2013-11-02T09:33:38Z","updated_at":"2016-07-30T21:40:47Z","pushed_at":"2016-07-31T16:37:16Z","git_url":"git://github.com/thoughtbot/gitsh.git","ssh_url":"git@github.com:thoughtbot/gitsh.git","clone_url":"https://github.com/thoughtbot/gitsh.git","svn_url":"https://github.com/thoughtbot/gitsh","homepage":"https://thoughtbot.com/tools","size":2036,"stargazers_count":1581,"watchers_count":1581,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":73,"mirror_url":null,"open_issues_count":27,"forks":73,"open_issues":27,"watchers":1581,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/thoughtbot/gitsh/pulls/264"},"html":{"href":"https://github.com/thoughtbot/gitsh/pull/264"},"issue":{"href":"https://api.github.com/repos/thoughtbot/gitsh/issues/264"},"comments":{"href":"https://api.github.com/repos/thoughtbot/gitsh/issues/264/comments"},"review_comments":{"href":"https://api.github.com/repos/thoughtbot/gitsh/pulls/264/comments"},"review_comment":{"href":"https://api.github.com/repos/thoughtbot/gitsh/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/thoughtbot/gitsh/pulls/264/commits"},"statuses":{"href":"https://api.github.com/repos/thoughtbot/gitsh/statuses/f1bba1e9238b0aa3d29ff7fdc996a7da5baeb554"}}}}
{"actor":{"display_login":"mike-burns"}}