GitHub PullRequestReviewCommentEvent at 2014-07-22 17:41:55
Repository: atom-latex
Actor: thomasjo
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/thomasjo/atom-latex/pulls/comments/15243252","id":15243252,"diff_hunk":"@@ -13,7 +14,8 @@ describe \"LatexmkBuilder\", ->\n \n   describe \"constructArgs\", ->\n     beforeEach ->\n-      @filePath = \"foo.tex\"\n+      @fixturesPath = path.join(atom.project.getPath(), \"master-tex-finder\", \"single-master\")","path":"spec/builders/latexmk-spec.coffee","position":11,"original_position":11,"commit_id":"8ce85d0b9f688e86d72341db884e2bacc9cdee2b","original_commit_id":"8ce85d0b9f688e86d72341db884e2bacc9cdee2b","user":{"login":"thomasjo","id":3622,"avatar_url":"https://avatars.githubusercontent.com/u/3622?","gravatar_id":"f15b7a13f34a0e8b9543c031b9bff538","url":"https://api.github.com/users/thomasjo","html_url":"https://github.com/thomasjo","followers_url":"https://api.github.com/users/thomasjo/followers","following_url":"https://api.github.com/users/thomasjo/following{/other_user}","gists_url":"https://api.github.com/users/thomasjo/gists{/gist_id}","starred_url":"https://api.github.com/users/thomasjo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thomasjo/subscriptions","organizations_url":"https://api.github.com/users/thomasjo/orgs","repos_url":"https://api.github.com/users/thomasjo/repos","events_url":"https://api.github.com/users/thomasjo/events{/privacy}","received_events_url":"https://api.github.com/users/thomasjo/received_events","type":"User","site_admin":false},"body":"I would really, really prefer if these changes very not necessary. I don't think they are, given the changes you've made in relation to short-circuiting the routine in `MasterTexFinder::masterTexPath`. All the old tests should be reverted to their original state and still pass.","created_at":"2014-07-22T17:41:55Z","updated_at":"2014-07-22T17:41:55Z","html_url":"https://github.com/thomasjo/atom-latex/pull/13#discussion_r15243252","pull_request_url":"https://api.github.com/repos/thomasjo/atom-latex/pulls/13","_links":{"self":{"href":"https://api.github.com/repos/thomasjo/atom-latex/pulls/comments/15243252"},"html":{"href":"https://github.com/thomasjo/atom-latex/pull/13#discussion_r15243252"},"pull_request":{"href":"https://api.github.com/repos/thomasjo/atom-latex/pulls/13"}}},"pull_request":{"url":"https://api.github.com/repos/thomasjo/atom-latex/pulls/13","id":18209925,"html_url":"https://github.com/thomasjo/atom-latex/pull/13","diff_url":"https://github.com/thomasjo/atom-latex/pull/13.diff","patch_url":"https://github.com/thomasjo/atom-latex/pull/13.patch","issue_url":"https://api.github.com/repos/thomasjo/atom-latex/issues/13","number":13,"state":"open","title":"Adds an auto-find-tex-master feature.","user":{"login":"boborbt","id":204476,"avatar_url":"https://avatars.githubusercontent.com/u/204476?","gravatar_id":"ad0444e322666dc2c8b78c1b6b8ca2b5","url":"https://api.github.com/users/boborbt","html_url":"https://github.com/boborbt","followers_url":"https://api.github.com/users/boborbt/followers","following_url":"https://api.github.com/users/boborbt/following{/other_user}","gists_url":"https://api.github.com/users/boborbt/gists{/gist_id}","starred_url":"https://api.github.com/users/boborbt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/boborbt/subscriptions","organizations_url":"https://api.github.com/users/boborbt/orgs","repos_url":"https://api.github.com/users/boborbt/repos","events_url":"https://api.github.com/users/boborbt/events{/privacy}","received_events_url":"https://api.github.com/users/boborbt/received_events","type":"User","site_admin":false},"body":"When asked to compile a latex file the package would now search the current directory for the master tex file. This is helpful in case the  latex document is split in several files since it allows compilation from any latex file in the project. IMHO this solves, except for weird project configurations, the most important problem currently deriving by still not having proper project management: the necessity to switch to the latex master file before invoking the compilation. The code in this pull request could be used when building project management support to initialise the project settings with a sensible guess of the latex master file. \r\n\r\nPerformance issues: the present implementation would  scan (possibly) every tex file contained in the current directory in search for a file containing a ```documentclass``` declaration. On my system (a relatively new MacBook pro) it adds negligible compilation time to a project with 10 latex files resulting in a 60 pages document (it also happens that the master file is found after analysing almost all the other files, so I was close to the worst case scenario). My gut feelings is that there should not be performance issues but for the most constrained machines working on the largest latex files. When proper project management is implemented, the issues would be solved since the search would be run only during the initialisation of the project (i.e., only once).\r\n\r\nPossible improvements: \r\n   - option to disable it (one should be able to set this option in the project and in the main package config);\r\n   - it may be useful to bind the compilation with this feature to a different key binding (in weird situations where a single directory contains more master files the algorithm would select one at random, -- notice that it is not hard to change the logic of the algorithm so to find the master file including the currently selected file).","created_at":"2014-07-10T09:51:29Z","updated_at":"2014-07-22T17:41:55Z","closed_at":null,"merged_at":null,"merge_commit_sha":"5de17369dee5a79696402c838d9b6eff0acf107e","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/thomasjo/atom-latex/pulls/13/commits","review_comments_url":"https://api.github.com/repos/thomasjo/atom-latex/pulls/13/comments","review_comment_url":"https://api.github.com/repos/thomasjo/atom-latex/pulls/comments/{number}","comments_url":"https://api.github.com/repos/thomasjo/atom-latex/issues/13/comments","statuses_url":"https://api.github.com/repos/thomasjo/atom-latex/statuses/8ce85d0b9f688e86d72341db884e2bacc9cdee2b","head":{"label":"boborbt:auto-find-tex-master","ref":"auto-find-tex-master","sha":"8ce85d0b9f688e86d72341db884e2bacc9cdee2b","user":{"login":"boborbt","id":204476,"avatar_url":"https://avatars.githubusercontent.com/u/204476?","gravatar_id":"ad0444e322666dc2c8b78c1b6b8ca2b5","url":"https://api.github.com/users/boborbt","html_url":"https://github.com/boborbt","followers_url":"https://api.github.com/users/boborbt/followers","following_url":"https://api.github.com/users/boborbt/following{/other_user}","gists_url":"https://api.github.com/users/boborbt/gists{/gist_id}","starred_url":"https://api.github.com/users/boborbt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/boborbt/subscriptions","organizations_url":"https://api.github.com/users/boborbt/orgs","repos_url":"https://api.github.com/users/boborbt/repos","events_url":"https://api.github.com/users/boborbt/events{/privacy}","received_events_url":"https://api.github.com/users/boborbt/received_events","type":"User","site_admin":false},"repo":{"id":21517462,"name":"atom-latex","full_name":"boborbt/atom-latex","owner":{"login":"boborbt","id":204476,"avatar_url":"https://avatars.githubusercontent.com/u/204476?","gravatar_id":"ad0444e322666dc2c8b78c1b6b8ca2b5","url":"https://api.github.com/users/boborbt","html_url":"https://github.com/boborbt","followers_url":"https://api.github.com/users/boborbt/followers","following_url":"https://api.github.com/users/boborbt/following{/other_user}","gists_url":"https://api.github.com/users/boborbt/gists{/gist_id}","starred_url":"https://api.github.com/users/boborbt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/boborbt/subscriptions","organizations_url":"https://api.github.com/users/boborbt/orgs","repos_url":"https://api.github.com/users/boborbt/repos","events_url":"https://api.github.com/users/boborbt/events{/privacy}","received_events_url":"https://api.github.com/users/boborbt/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/boborbt/atom-latex","description":"A simple utility package for Atom that invokes latexmk.","fork":true,"url":"https://api.github.com/repos/boborbt/atom-latex","forks_url":"https://api.github.com/repos/boborbt/atom-latex/forks","keys_url":"https://api.github.com/repos/boborbt/atom-latex/keys{/key_id}","collaborators_url":"https://api.github.com/repos/boborbt/atom-latex/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/boborbt/atom-latex/teams","hooks_url":"https://api.github.com/repos/boborbt/atom-latex/hooks","issue_events_url":"https://api.github.com/repos/boborbt/atom-latex/issues/events{/number}","events_url":"https://api.github.com/repos/boborbt/atom-latex/events","assignees_url":"https://api.github.com/repos/boborbt/atom-latex/assignees{/user}","branches_url":"https://api.github.com/repos/boborbt/atom-latex/branches{/branch}","tags_url":"https://api.github.com/repos/boborbt/atom-latex/tags","blobs_url":"https://api.github.com/repos/boborbt/atom-latex/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/boborbt/atom-latex/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/boborbt/atom-latex/git/refs{/sha}","trees_url":"https://api.github.com/repos/boborbt/atom-latex/git/trees{/sha}","statuses_url":"https://api.github.com/repos/boborbt/atom-latex/statuses/{sha}","languages_url":"https://api.github.com/repos/boborbt/atom-latex/languages","stargazers_url":"https://api.github.com/repos/boborbt/atom-latex/stargazers","contributors_url":"https://api.github.com/repos/boborbt/atom-latex/contributors","subscribers_url":"https://api.github.com/repos/boborbt/atom-latex/subscribers","subscription_url":"https://api.github.com/repos/boborbt/atom-latex/subscription","commits_url":"https://api.github.com/repos/boborbt/atom-latex/commits{/sha}","git_commits_url":"https://api.github.com/repos/boborbt/atom-latex/git/commits{/sha}","comments_url":"https://api.github.com/repos/boborbt/atom-latex/comments{/number}","issue_comment_url":"https://api.github.com/repos/boborbt/atom-latex/issues/comments/{number}","contents_url":"https://api.github.com/repos/boborbt/atom-latex/contents/{+path}","compare_url":"https://api.github.com/repos/boborbt/atom-latex/compare/{base}...{head}","merges_url":"https://api.github.com/repos/boborbt/atom-latex/merges","archive_url":"https://api.github.com/repos/boborbt/atom-latex/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/boborbt/atom-latex/downloads","issues_url":"https://api.github.com/repos/boborbt/atom-latex/issues{/number}","pulls_url":"https://api.github.com/repos/boborbt/atom-latex/pulls{/number}","milestones_url":"https://api.github.com/repos/boborbt/atom-latex/milestones{/number}","notifications_url":"https://api.github.com/repos/boborbt/atom-latex/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/boborbt/atom-latex/labels{/name}","releases_url":"https://api.github.com/repos/boborbt/atom-latex/releases{/id}","created_at":"2014-07-05T10:04:57Z","updated_at":"2014-07-05T10:06:01Z","pushed_at":"2014-07-22T12:32:33Z","git_url":"git://github.com/boborbt/atom-latex.git","ssh_url":"git@github.com:boborbt/atom-latex.git","clone_url":"https://github.com/boborbt/atom-latex.git","svn_url":"https://github.com/boborbt/atom-latex","homepage":"https://atom.io/packages/latex","size":226,"stargazers_count":0,"watchers_count":0,"language":"CoffeeScript","has_issues":false,"has_downloads":true,"has_wiki":true,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"thomasjo:master","ref":"master","sha":"31796826d53f2148fb646325a01c5740cfd3907a","user":{"login":"thomasjo","id":3622,"avatar_url":"https://avatars.githubusercontent.com/u/3622?","gravatar_id":"f15b7a13f34a0e8b9543c031b9bff538","url":"https://api.github.com/users/thomasjo","html_url":"https://github.com/thomasjo","followers_url":"https://api.github.com/users/thomasjo/followers","following_url":"https://api.github.com/users/thomasjo/following{/other_user}","gists_url":"https://api.github.com/users/thomasjo/gists{/gist_id}","starred_url":"https://api.github.com/users/thomasjo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thomasjo/subscriptions","organizations_url":"https://api.github.com/users/thomasjo/orgs","repos_url":"https://api.github.com/users/thomasjo/repos","events_url":"https://api.github.com/users/thomasjo/events{/privacy}","received_events_url":"https://api.github.com/users/thomasjo/received_events","type":"User","site_admin":false},"repo":{"id":17474966,"name":"atom-latex","full_name":"thomasjo/atom-latex","owner":{"login":"thomasjo","id":3622,"avatar_url":"https://avatars.githubusercontent.com/u/3622?","gravatar_id":"f15b7a13f34a0e8b9543c031b9bff538","url":"https://api.github.com/users/thomasjo","html_url":"https://github.com/thomasjo","followers_url":"https://api.github.com/users/thomasjo/followers","following_url":"https://api.github.com/users/thomasjo/following{/other_user}","gists_url":"https://api.github.com/users/thomasjo/gists{/gist_id}","starred_url":"https://api.github.com/users/thomasjo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thomasjo/subscriptions","organizations_url":"https://api.github.com/users/thomasjo/orgs","repos_url":"https://api.github.com/users/thomasjo/repos","events_url":"https://api.github.com/users/thomasjo/events{/privacy}","received_events_url":"https://api.github.com/users/thomasjo/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/thomasjo/atom-latex","description":"A simple utility package for Atom that invokes latexmk.","fork":false,"url":"https://api.github.com/repos/thomasjo/atom-latex","forks_url":"https://api.github.com/repos/thomasjo/atom-latex/forks","keys_url":"https://api.github.com/repos/thomasjo/atom-latex/keys{/key_id}","collaborators_url":"https://api.github.com/repos/thomasjo/atom-latex/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/thomasjo/atom-latex/teams","hooks_url":"https://api.github.com/repos/thomasjo/atom-latex/hooks","issue_events_url":"https://api.github.com/repos/thomasjo/atom-latex/issues/events{/number}","events_url":"https://api.github.com/repos/thomasjo/atom-latex/events","assignees_url":"https://api.github.com/repos/thomasjo/atom-latex/assignees{/user}","branches_url":"https://api.github.com/repos/thomasjo/atom-latex/branches{/branch}","tags_url":"https://api.github.com/repos/thomasjo/atom-latex/tags","blobs_url":"https://api.github.com/repos/thomasjo/atom-latex/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/thomasjo/atom-latex/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/thomasjo/atom-latex/git/refs{/sha}","trees_url":"https://api.github.com/repos/thomasjo/atom-latex/git/trees{/sha}","statuses_url":"https://api.github.com/repos/thomasjo/atom-latex/statuses/{sha}","languages_url":"https://api.github.com/repos/thomasjo/atom-latex/languages","stargazers_url":"https://api.github.com/repos/thomasjo/atom-latex/stargazers","contributors_url":"https://api.github.com/repos/thomasjo/atom-latex/contributors","subscribers_url":"https://api.github.com/repos/thomasjo/atom-latex/subscribers","subscription_url":"https://api.github.com/repos/thomasjo/atom-latex/subscription","commits_url":"https://api.github.com/repos/thomasjo/atom-latex/commits{/sha}","git_commits_url":"https://api.github.com/repos/thomasjo/atom-latex/git/commits{/sha}","comments_url":"https://api.github.com/repos/thomasjo/atom-latex/comments{/number}","issue_comment_url":"https://api.github.com/repos/thomasjo/atom-latex/issues/comments/{number}","contents_url":"https://api.github.com/repos/thomasjo/atom-latex/contents/{+path}","compare_url":"https://api.github.com/repos/thomasjo/atom-latex/compare/{base}...{head}","merges_url":"https://api.github.com/repos/thomasjo/atom-latex/merges","archive_url":"https://api.github.com/repos/thomasjo/atom-latex/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/thomasjo/atom-latex/downloads","issues_url":"https://api.github.com/repos/thomasjo/atom-latex/issues{/number}","pulls_url":"https://api.github.com/repos/thomasjo/atom-latex/pulls{/number}","milestones_url":"https://api.github.com/repos/thomasjo/atom-latex/milestones{/number}","notifications_url":"https://api.github.com/repos/thomasjo/atom-latex/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/thomasjo/atom-latex/labels{/name}","releases_url":"https://api.github.com/repos/thomasjo/atom-latex/releases{/id}","created_at":"2014-03-06T11:03:55Z","updated_at":"2014-07-16T13:04:34Z","pushed_at":"2014-07-16T16:13:07Z","git_url":"git://github.com/thomasjo/atom-latex.git","ssh_url":"git@github.com:thomasjo/atom-latex.git","clone_url":"https://github.com/thomasjo/atom-latex.git","svn_url":"https://github.com/thomasjo/atom-latex","homepage":"https://atom.io/packages/latex","size":771,"stargazers_count":11,"watchers_count":11,"language":"CoffeeScript","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":5,"mirror_url":null,"open_issues_count":6,"forks":5,"open_issues":6,"watchers":11,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/thomasjo/atom-latex/pulls/13"},"html":{"href":"https://github.com/thomasjo/atom-latex/pull/13"},"issue":{"href":"https://api.github.com/repos/thomasjo/atom-latex/issues/13"},"comments":{"href":"https://api.github.com/repos/thomasjo/atom-latex/issues/13/comments"},"review_comments":{"href":"https://api.github.com/repos/thomasjo/atom-latex/pulls/13/comments"},"review_comment":{"href":"https://api.github.com/repos/thomasjo/atom-latex/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/thomasjo/atom-latex/pulls/13/commits"},"statuses":{"href":"https://api.github.com/repos/thomasjo/atom-latex/statuses/8ce85d0b9f688e86d72341db884e2bacc9cdee2b"}}}}