{"action":"created","issue":{"url":"https://api.github.com/repos/processing-js/processing-js/issues/278","repository_url":"https://api.github.com/repos/processing-js/processing-js","labels_url":"https://api.github.com/repos/processing-js/processing-js/issues/278/labels{/name}","comments_url":"https://api.github.com/repos/processing-js/processing-js/issues/278/comments","events_url":"https://api.github.com/repos/processing-js/processing-js/issues/278/events","html_url":"https://github.com/processing-js/processing-js/pull/278","id":211904940,"number":278,"title":"Fix issue #271 - hue value -1 causes crash","user":{"login":"ojcm","id":7442246,"avatar_url":"https://avatars3.githubusercontent.com/u/7442246?v=3","gravatar_id":"","url":"https://api.github.com/users/ojcm","html_url":"https://github.com/ojcm","followers_url":"https://api.github.com/users/ojcm/followers","following_url":"https://api.github.com/users/ojcm/following{/other_user}","gists_url":"https://api.github.com/users/ojcm/gists{/gist_id}","starred_url":"https://api.github.com/users/ojcm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ojcm/subscriptions","organizations_url":"https://api.github.com/users/ojcm/orgs","repos_url":"https://api.github.com/users/ojcm/repos","events_url":"https://api.github.com/users/ojcm/events{/privacy}","received_events_url":"https://api.github.com/users/ojcm/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":8,"created_at":"2017-03-04T20:29:42Z","updated_at":"2017-03-05T19:25:54Z","closed_at":"2017-03-05T18:09:18Z","pull_request":{"url":"https://api.github.com/repos/processing-js/processing-js/pulls/278","html_url":"https://github.com/processing-js/processing-js/pull/278","diff_url":"https://github.com/processing-js/processing-js/pull/278.diff","patch_url":"https://github.com/processing-js/processing-js/pull/278.patch"},"body":"Hue should be in range 0-255. Processing behaviour when given a hue value below '0' appears to be to use '0'. To replicate that behaviour and fix this issue we should force the hue value to 0 when it is a negative value.\r\n\r\nThe specific error the user saw was caused by switching on '-1' (the value of `Math.floor(hue / 60)` when h is -1) in function `toRGB`. As there is no case for '-1', the function didn't return anything. The function `color` then sets `r = rgb[0];`, but rgb is undefined, hence the error \"Cannot read property '0' of undefined\". \r\nTo provide better diagnostics in future I've added a default case to the switch, this includes a debugging statement indicating that this case is unexpected.\r\n\r\nUnit test confirms that negative hue value is treated as '0'.\r\n\r\nFixes #271 "},"comment":{"url":"https://api.github.com/repos/processing-js/processing-js/issues/comments/284253370","html_url":"https://github.com/processing-js/processing-js/pull/278#issuecomment-284253370","issue_url":"https://api.github.com/repos/processing-js/processing-js/issues/278","id":284253370,"user":{"login":"Pomax","id":177243,"avatar_url":"https://avatars1.githubusercontent.com/u/177243?v=3","gravatar_id":"","url":"https://api.github.com/users/Pomax","html_url":"https://github.com/Pomax","followers_url":"https://api.github.com/users/Pomax/followers","following_url":"https://api.github.com/users/Pomax/following{/other_user}","gists_url":"https://api.github.com/users/Pomax/gists{/gist_id}","starred_url":"https://api.github.com/users/Pomax/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Pomax/subscriptions","organizations_url":"https://api.github.com/users/Pomax/orgs","repos_url":"https://api.github.com/users/Pomax/repos","events_url":"https://api.github.com/users/Pomax/events{/privacy}","received_events_url":"https://api.github.com/users/Pomax/received_events","type":"User","site_admin":false},"created_at":"2017-03-05T19:25:54Z","updated_at":"2017-03-05T19:25:54Z","body":"that logic doesn't hold though. There is no reason whatsoever to add safety checks around all parameters just because it was added to one. If one of them can cause a crash and the others can't, then fixing just the crash is perfectly fine without any need for additional work until there is an actual use case in which normal code causes erroneous behaviour.\r\n\r\nAlso note that there is zero requirement for one PR to cover multiple failures. If s<0 or b<0 is something Processing allows, and the reference docs don't say anything about \"these values must be positive numbers\" or some such, then filing multiple PRs to successively get to parity is perfectly acceptable.\r\n\r\nAfter all, just because one PR lands doesn't mean no further fixes will ever be accepted for that codepath. And if they're subtly different issues, for instance `h` causing a crash, whereas `s`/`b` are merely not on par, then those should be two separate issues and working on them in two different PRs is certainly preferred."}}
{"actor":{"display_login":"Pomax"}}