GitHub PullRequestReviewCommentEvent at 2014-08-01 18:36:32
Repository: pexpect
Actor: takluyver
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/pexpect/pexpect/pulls/comments/15711651","id":15711651,"diff_hunk":"@@ -147,6 +150,62 @@ def test_number_x(self):\n         assert str(s) == ('test                ')\n         assert(s.state.memory == [s, '0', '1', '32', '45'])\n \n+    def test_utf8_bytes(self):\n+        \"\"\"Test that when bytes are passed in containing UTF-8 encoded\n+        characters, where the encoding of each character consists of\n+        multiple bytes, the characters are correctly decoded.\n+        Incremental decoding is also tested.\"\"\"\n+        s = ANSI.ANSI(2, 10, encoding='utf-8')\n+        # This is the UTF-8 encoding of the UCS character \"HOURGLASS\"\n+        # followed by the UTF-8 encoding of the UCS character\n+        # \"KEYBOARD\".  These characters can't be encoded in cp437 or\n+        # latin-1.  The \"KEYBOARD\" character is split into two\n+        # separate writes.\n+        s.write(b'\\xe2\\x8c\\x9b')\n+        s.write(b'\\xe2\\x8c')\n+        s.write(b'\\xa8')\n+        if PY3:\n+            assert str(s) == u'\\u231b\\u2328        \\n          '\n+        else:\n+            assert unicode(s) == u'\\u231b\\u2328        \\n          '\n+            assert str(s) == b'\\xe2\\x8c\\x9b\\xe2\\x8c\\xa8        \\n          '","path":"tests/test_ansi.py","position":32,"original_position":32,"commit_id":"a4b8165da2a7f356e3a12b091727e33e6fcc0d2c","original_commit_id":"a4b8165da2a7f356e3a12b091727e33e6fcc0d2c","user":{"login":"takluyver","id":327925,"avatar_url":"https://avatars.githubusercontent.com/u/327925?v=1","gravatar_id":"7f1ae3f14fd5b87c2c3f7b36014e185c","url":"https://api.github.com/users/takluyver","html_url":"https://github.com/takluyver","followers_url":"https://api.github.com/users/takluyver/followers","following_url":"https://api.github.com/users/takluyver/following{/other_user}","gists_url":"https://api.github.com/users/takluyver/gists{/gist_id}","starred_url":"https://api.github.com/users/takluyver/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/takluyver/subscriptions","organizations_url":"https://api.github.com/users/takluyver/orgs","repos_url":"https://api.github.com/users/takluyver/repos","events_url":"https://api.github.com/users/takluyver/events{/privacy}","received_events_url":"https://api.github.com/users/takluyver/received_events","type":"User","site_admin":false},"body":"...but that's the kind of thing that I think the author gets to decide about, so if you think it's better this way, that's OK.","created_at":"2014-08-01T18:36:32Z","updated_at":"2014-08-01T18:36:32Z","html_url":"https://github.com/pexpect/pexpect/pull/96#discussion_r15711651","pull_request_url":"https://api.github.com/repos/pexpect/pexpect/pulls/96","_links":{"self":{"href":"https://api.github.com/repos/pexpect/pexpect/pulls/comments/15711651"},"html":{"href":"https://github.com/pexpect/pexpect/pull/96#discussion_r15711651"},"pull_request":{"href":"https://api.github.com/repos/pexpect/pexpect/pulls/96"}}},"pull_request":{"url":"https://api.github.com/repos/pexpect/pexpect/pulls/96","id":18834174,"html_url":"https://github.com/pexpect/pexpect/pull/96","diff_url":"https://github.com/pexpect/pexpect/pull/96.diff","patch_url":"https://github.com/pexpect/pexpect/pull/96.patch","issue_url":"https://api.github.com/repos/pexpect/pexpect/issues/96","number":96,"state":"open","title":"Issue #84: Unicode support in screen and ANSI.","user":{"login":"dcoshea","id":2437134,"avatar_url":"https://avatars.githubusercontent.com/u/2437134?v=1","gravatar_id":"c9f21fe8478dd06a43cba6dd810d2691","url":"https://api.github.com/users/dcoshea","html_url":"https://github.com/dcoshea","followers_url":"https://api.github.com/users/dcoshea/followers","following_url":"https://api.github.com/users/dcoshea/following{/other_user}","gists_url":"https://api.github.com/users/dcoshea/gists{/gist_id}","starred_url":"https://api.github.com/users/dcoshea/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dcoshea/subscriptions","organizations_url":"https://api.github.com/users/dcoshea/orgs","repos_url":"https://api.github.com/users/dcoshea/repos","events_url":"https://api.github.com/users/dcoshea/events{/privacy}","received_events_url":"https://api.github.com/users/dcoshea/received_events","type":"User","site_admin":false},"body":"Please see the commit message for details.\r\n\r\nI'm not sure if I'm on the right track at all, so I didn't spend a huge amount of time trying to tidy the code, get the comments exactly right, etc.  I'm happy to fix that stuff up if I'm on the right track.  Apologies if I'm on the completely wrong track!\r\n\r\nThe screen and ANSI unit tests pass on Python 2.6, 2.7 and 3.3.\r\n\r\nSome issues I'm aware of which I think are fairly minor:\r\n\r\nIt's not 100% clear to me which methods are meant to be public and\r\nwhich are only meant for internal use, so I've tried to perform\r\ndecoding in all methods that accept input.\r\n\r\nThere are some cases (e.g. screen.fill(), fill_region(), put(),\r\ninsert_abs(), insert()) where decoding is performed by a method that\r\nthen invokes another method that also potentially performs decoding.\r\nI thought that this was safer as it meant that future refactoring\r\nwouldn't inadvertently result in decoding not being performed in some\r\ncode path.\r\n\r\nUse of the incremental decoder from ANSI.write_ch() and\r\nscreen.put_abs() technically doesn't make sense, since those methods\r\nexpect to receive exactly one character, and if the caller actually\r\nneeds an incremental decoder to be used because they don't know where\r\nthe character boundaries are in the byte stream, they shouldn't be\r\ncalling those methods.  However, I've just used the same incremental\r\ndecoder in those methods as in all the other ones.  I assume those\r\nmethods probably wouldn't typically be called directly by outside\r\ncode, and if they were, the user would have to have more of an idea of\r\nwhat they were doing with regards to multi-byte encodings.\r\n\r\nWith an incremental decoder, we're meant to call it once at the end\r\nwith final=True so that any incomplete byte sequence at the end of the\r\nstream can be detected.  I don't know of a sensible way to do that -\r\nperhaps screen() should have a close() method added to it that does\r\nthis?","created_at":"2014-07-24T12:37:40Z","updated_at":"2014-08-01T18:36:32Z","closed_at":null,"merged_at":null,"merge_commit_sha":"53daa136cd4c9085f60b7dcadedb84a64bf69a0d","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/pexpect/pexpect/pulls/96/commits","review_comments_url":"https://api.github.com/repos/pexpect/pexpect/pulls/96/comments","review_comment_url":"https://api.github.com/repos/pexpect/pexpect/pulls/comments/{number}","comments_url":"https://api.github.com/repos/pexpect/pexpect/issues/96/comments","statuses_url":"https://api.github.com/repos/pexpect/pexpect/statuses/a4b8165da2a7f356e3a12b091727e33e6fcc0d2c","head":{"label":"dcoshea:issue-84","ref":"issue-84","sha":"a4b8165da2a7f356e3a12b091727e33e6fcc0d2c","user":{"login":"dcoshea","id":2437134,"avatar_url":"https://avatars.githubusercontent.com/u/2437134?v=1","gravatar_id":"c9f21fe8478dd06a43cba6dd810d2691","url":"https://api.github.com/users/dcoshea","html_url":"https://github.com/dcoshea","followers_url":"https://api.github.com/users/dcoshea/followers","following_url":"https://api.github.com/users/dcoshea/following{/other_user}","gists_url":"https://api.github.com/users/dcoshea/gists{/gist_id}","starred_url":"https://api.github.com/users/dcoshea/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dcoshea/subscriptions","organizations_url":"https://api.github.com/users/dcoshea/orgs","repos_url":"https://api.github.com/users/dcoshea/repos","events_url":"https://api.github.com/users/dcoshea/events{/privacy}","received_events_url":"https://api.github.com/users/dcoshea/received_events","type":"User","site_admin":false},"repo":{"id":21450788,"name":"pexpect","full_name":"dcoshea/pexpect","owner":{"login":"dcoshea","id":2437134,"avatar_url":"https://avatars.githubusercontent.com/u/2437134?v=1","gravatar_id":"c9f21fe8478dd06a43cba6dd810d2691","url":"https://api.github.com/users/dcoshea","html_url":"https://github.com/dcoshea","followers_url":"https://api.github.com/users/dcoshea/followers","following_url":"https://api.github.com/users/dcoshea/following{/other_user}","gists_url":"https://api.github.com/users/dcoshea/gists{/gist_id}","starred_url":"https://api.github.com/users/dcoshea/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dcoshea/subscriptions","organizations_url":"https://api.github.com/users/dcoshea/orgs","repos_url":"https://api.github.com/users/dcoshea/repos","events_url":"https://api.github.com/users/dcoshea/events{/privacy}","received_events_url":"https://api.github.com/users/dcoshea/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/dcoshea/pexpect","description":"A Python module for controlling interactive programs in a pseudo-terminal","fork":true,"url":"https://api.github.com/repos/dcoshea/pexpect","forks_url":"https://api.github.com/repos/dcoshea/pexpect/forks","keys_url":"https://api.github.com/repos/dcoshea/pexpect/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dcoshea/pexpect/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dcoshea/pexpect/teams","hooks_url":"https://api.github.com/repos/dcoshea/pexpect/hooks","issue_events_url":"https://api.github.com/repos/dcoshea/pexpect/issues/events{/number}","events_url":"https://api.github.com/repos/dcoshea/pexpect/events","assignees_url":"https://api.github.com/repos/dcoshea/pexpect/assignees{/user}","branches_url":"https://api.github.com/repos/dcoshea/pexpect/branches{/branch}","tags_url":"https://api.github.com/repos/dcoshea/pexpect/tags","blobs_url":"https://api.github.com/repos/dcoshea/pexpect/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dcoshea/pexpect/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dcoshea/pexpect/git/refs{/sha}","trees_url":"https://api.github.com/repos/dcoshea/pexpect/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dcoshea/pexpect/statuses/{sha}","languages_url":"https://api.github.com/repos/dcoshea/pexpect/languages","stargazers_url":"https://api.github.com/repos/dcoshea/pexpect/stargazers","contributors_url":"https://api.github.com/repos/dcoshea/pexpect/contributors","subscribers_url":"https://api.github.com/repos/dcoshea/pexpect/subscribers","subscription_url":"https://api.github.com/repos/dcoshea/pexpect/subscription","commits_url":"https://api.github.com/repos/dcoshea/pexpect/commits{/sha}","git_commits_url":"https://api.github.com/repos/dcoshea/pexpect/git/commits{/sha}","comments_url":"https://api.github.com/repos/dcoshea/pexpect/comments{/number}","issue_comment_url":"https://api.github.com/repos/dcoshea/pexpect/issues/comments/{number}","contents_url":"https://api.github.com/repos/dcoshea/pexpect/contents/{+path}","compare_url":"https://api.github.com/repos/dcoshea/pexpect/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dcoshea/pexpect/merges","archive_url":"https://api.github.com/repos/dcoshea/pexpect/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dcoshea/pexpect/downloads","issues_url":"https://api.github.com/repos/dcoshea/pexpect/issues{/number}","pulls_url":"https://api.github.com/repos/dcoshea/pexpect/pulls{/number}","milestones_url":"https://api.github.com/repos/dcoshea/pexpect/milestones{/number}","notifications_url":"https://api.github.com/repos/dcoshea/pexpect/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dcoshea/pexpect/labels{/name}","releases_url":"https://api.github.com/repos/dcoshea/pexpect/releases{/id}","created_at":"2014-07-03T05:25:44Z","updated_at":"2014-07-08T01:23:49Z","pushed_at":"2014-08-01T08:10:43Z","git_url":"git://github.com/dcoshea/pexpect.git","ssh_url":"git@github.com:dcoshea/pexpect.git","clone_url":"https://github.com/dcoshea/pexpect.git","svn_url":"https://github.com/dcoshea/pexpect","homepage":"http://pexpect.readthedocs.org/","size":8560,"stargazers_count":0,"watchers_count":0,"language":"Python","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":"pexpect:master","ref":"master","sha":"3c9644f1cd6af755517e730b8e712e3026b4628c","user":{"login":"pexpect","id":5480175,"avatar_url":"https://avatars.githubusercontent.com/u/5480175?v=1","gravatar_id":null,"url":"https://api.github.com/users/pexpect","html_url":"https://github.com/pexpect","followers_url":"https://api.github.com/users/pexpect/followers","following_url":"https://api.github.com/users/pexpect/following{/other_user}","gists_url":"https://api.github.com/users/pexpect/gists{/gist_id}","starred_url":"https://api.github.com/users/pexpect/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pexpect/subscriptions","organizations_url":"https://api.github.com/users/pexpect/orgs","repos_url":"https://api.github.com/users/pexpect/repos","events_url":"https://api.github.com/users/pexpect/events{/privacy}","received_events_url":"https://api.github.com/users/pexpect/received_events","type":"Organization","site_admin":false},"repo":{"id":12902206,"name":"pexpect","full_name":"pexpect/pexpect","owner":{"login":"pexpect","id":5480175,"avatar_url":"https://avatars.githubusercontent.com/u/5480175?v=1","gravatar_id":null,"url":"https://api.github.com/users/pexpect","html_url":"https://github.com/pexpect","followers_url":"https://api.github.com/users/pexpect/followers","following_url":"https://api.github.com/users/pexpect/following{/other_user}","gists_url":"https://api.github.com/users/pexpect/gists{/gist_id}","starred_url":"https://api.github.com/users/pexpect/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pexpect/subscriptions","organizations_url":"https://api.github.com/users/pexpect/orgs","repos_url":"https://api.github.com/users/pexpect/repos","events_url":"https://api.github.com/users/pexpect/events{/privacy}","received_events_url":"https://api.github.com/users/pexpect/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/pexpect/pexpect","description":"A Python module for controlling interactive programs in a pseudo-terminal","fork":false,"url":"https://api.github.com/repos/pexpect/pexpect","forks_url":"https://api.github.com/repos/pexpect/pexpect/forks","keys_url":"https://api.github.com/repos/pexpect/pexpect/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pexpect/pexpect/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pexpect/pexpect/teams","hooks_url":"https://api.github.com/repos/pexpect/pexpect/hooks","issue_events_url":"https://api.github.com/repos/pexpect/pexpect/issues/events{/number}","events_url":"https://api.github.com/repos/pexpect/pexpect/events","assignees_url":"https://api.github.com/repos/pexpect/pexpect/assignees{/user}","branches_url":"https://api.github.com/repos/pexpect/pexpect/branches{/branch}","tags_url":"https://api.github.com/repos/pexpect/pexpect/tags","blobs_url":"https://api.github.com/repos/pexpect/pexpect/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pexpect/pexpect/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pexpect/pexpect/git/refs{/sha}","trees_url":"https://api.github.com/repos/pexpect/pexpect/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pexpect/pexpect/statuses/{sha}","languages_url":"https://api.github.com/repos/pexpect/pexpect/languages","stargazers_url":"https://api.github.com/repos/pexpect/pexpect/stargazers","contributors_url":"https://api.github.com/repos/pexpect/pexpect/contributors","subscribers_url":"https://api.github.com/repos/pexpect/pexpect/subscribers","subscription_url":"https://api.github.com/repos/pexpect/pexpect/subscription","commits_url":"https://api.github.com/repos/pexpect/pexpect/commits{/sha}","git_commits_url":"https://api.github.com/repos/pexpect/pexpect/git/commits{/sha}","comments_url":"https://api.github.com/repos/pexpect/pexpect/comments{/number}","issue_comment_url":"https://api.github.com/repos/pexpect/pexpect/issues/comments/{number}","contents_url":"https://api.github.com/repos/pexpect/pexpect/contents/{+path}","compare_url":"https://api.github.com/repos/pexpect/pexpect/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pexpect/pexpect/merges","archive_url":"https://api.github.com/repos/pexpect/pexpect/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pexpect/pexpect/downloads","issues_url":"https://api.github.com/repos/pexpect/pexpect/issues{/number}","pulls_url":"https://api.github.com/repos/pexpect/pexpect/pulls{/number}","milestones_url":"https://api.github.com/repos/pexpect/pexpect/milestones{/number}","notifications_url":"https://api.github.com/repos/pexpect/pexpect/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pexpect/pexpect/labels{/name}","releases_url":"https://api.github.com/repos/pexpect/pexpect/releases{/id}","created_at":"2013-09-17T17:36:44Z","updated_at":"2014-07-24T12:54:01Z","pushed_at":"2014-07-22T16:59:40Z","git_url":"git://github.com/pexpect/pexpect.git","ssh_url":"git@github.com:pexpect/pexpect.git","clone_url":"https://github.com/pexpect/pexpect.git","svn_url":"https://github.com/pexpect/pexpect","homepage":"http://pexpect.readthedocs.org/","size":11273,"stargazers_count":129,"watchers_count":129,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":36,"mirror_url":null,"open_issues_count":29,"forks":36,"open_issues":29,"watchers":129,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/pexpect/pexpect/pulls/96"},"html":{"href":"https://github.com/pexpect/pexpect/pull/96"},"issue":{"href":"https://api.github.com/repos/pexpect/pexpect/issues/96"},"comments":{"href":"https://api.github.com/repos/pexpect/pexpect/issues/96/comments"},"review_comments":{"href":"https://api.github.com/repos/pexpect/pexpect/pulls/96/comments"},"review_comment":{"href":"https://api.github.com/repos/pexpect/pexpect/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/pexpect/pexpect/pulls/96/commits"},"statuses":{"href":"https://api.github.com/repos/pexpect/pexpect/statuses/a4b8165da2a7f356e3a12b091727e33e6fcc0d2c"}}}}