{"action":"created","issue":{"url":"https://api.github.com/repos/cjheath/treetop/issues/26","labels_url":"https://api.github.com/repos/cjheath/treetop/issues/26/labels{/name}","comments_url":"https://api.github.com/repos/cjheath/treetop/issues/26/comments","events_url":"https://api.github.com/repos/cjheath/treetop/issues/26/events","html_url":"https://github.com/cjheath/treetop/pull/26","id":85958161,"number":26,"title":"Store terminal failures as TerminalParseFailures","user":{"login":"sds","id":677877,"avatar_url":"https://avatars.githubusercontent.com/u/677877?v=3","gravatar_id":"","url":"https://api.github.com/users/sds","html_url":"https://github.com/sds","followers_url":"https://api.github.com/users/sds/followers","following_url":"https://api.github.com/users/sds/following{/other_user}","gists_url":"https://api.github.com/users/sds/gists{/gist_id}","starred_url":"https://api.github.com/users/sds/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sds/subscriptions","organizations_url":"https://api.github.com/users/sds/orgs","repos_url":"https://api.github.com/users/sds/repos","events_url":"https://api.github.com/users/sds/events{/privacy}","received_events_url":"https://api.github.com/users/sds/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-06-07T22:43:42Z","updated_at":"2015-06-09T06:06:35Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cjheath/treetop/pulls/26","html_url":"https://github.com/cjheath/treetop/pull/26","diff_url":"https://github.com/cjheath/treetop/pull/26.diff","patch_url":"https://github.com/cjheath/treetop/pull/26.patch"},"body":"When attempting to print out a failure message using `failure_reason`\r\nfor my tiny personal project `propose`[1], I was encountering the following error:\r\n\r\n    NoMethodError: undefined method `unexpected' for [0, \"[a-z]\", false]:Array> with backtrace\r\n\r\nThis started happening after upgrading to `treetop` 1.6.0+.\r\n\r\nIt looks like the problem here is that terminal failures are stored in\r\n`@terminal_failures` as tuples, and when you call `terminal_failures` it\r\ndoes a check to see if the first item is a `TerminalParseFailures`, and\r\nconverts the list if it isn't. However, if you call `terminal_failures`\r\nmultiple times, it's possible other errors were added, and so you'll\r\nhave a mixture of `Array` tuples and `TerminalParseFailure`s.\r\n\r\nThe fix is to store them as `TerminalParseFailure`s when the failure is\r\nfirst encountered in `terminal_parse_failure`. It's not overly expensive\r\nand allows us to avoid this unnecessary complexity.\r\n\r\n[1] https://github.com/sds/propose"},"comment":{"url":"https://api.github.com/repos/cjheath/treetop/issues/comments/110238934","html_url":"https://github.com/cjheath/treetop/pull/26#issuecomment-110238934","issue_url":"https://api.github.com/repos/cjheath/treetop/issues/26","id":110238934,"user":{"login":"cjheath","id":1703,"avatar_url":"https://avatars.githubusercontent.com/u/1703?v=3","gravatar_id":"","url":"https://api.github.com/users/cjheath","html_url":"https://github.com/cjheath","followers_url":"https://api.github.com/users/cjheath/followers","following_url":"https://api.github.com/users/cjheath/following{/other_user}","gists_url":"https://api.github.com/users/cjheath/gists{/gist_id}","starred_url":"https://api.github.com/users/cjheath/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cjheath/subscriptions","organizations_url":"https://api.github.com/users/cjheath/orgs","repos_url":"https://api.github.com/users/cjheath/repos","events_url":"https://api.github.com/users/cjheath/events{/privacy}","received_events_url":"https://api.github.com/users/cjheath/received_events","type":"User","site_admin":false},"created_at":"2015-06-09T06:06:35Z","updated_at":"2015-06-09T06:06:35Z","body":"Not opposed, just want to understand so as to avoid fixing what's not broken.\r\nIn the early days, TerminalParseFailures were being created for every failed terminal,\r\nnow we only create them when we're at the right-most point we've reached in the input,\r\nand that saved a huge amount of time and memory. Now it's creating the Array tuple and\r\nconverting it later, which probably isn't really a saving; except perhaps that Arrays are\r\noptimised in the Ruby runtime and might be quicker to create than the TPF. I don't know,\r\nas AFAIK there's been no comparative performance testing done in some years and a lot\r\nof things have changed in the Ruby runtimes.\r\n\r\nI hope that explains the (historical) rationale anyhow. Whatever makes sense now will\r\nbe acceptable, as long as we understand why the change is needed. So yes, if you\r\ncould find time to investigate, I'd like to know."}}