{"action":"created","issue":{"url":"https://api.github.com/repos/MOcov/MOcov/issues/14","repository_url":"https://api.github.com/repos/MOcov/MOcov","labels_url":"https://api.github.com/repos/MOcov/MOcov/issues/14/labels{/name}","comments_url":"https://api.github.com/repos/MOcov/MOcov/issues/14/comments","events_url":"https://api.github.com/repos/MOcov/MOcov/issues/14/events","html_url":"https://github.com/MOcov/MOcov/pull/14","id":333031904,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk1MzU5Mjk4","number":14,"title":"WIP: better multiline command support, multiple root_dirs","user":{"login":"noelbk","id":4550655,"node_id":"MDQ6VXNlcjQ1NTA2NTU=","avatar_url":"https://avatars3.githubusercontent.com/u/4550655?v=4","gravatar_id":"","url":"https://api.github.com/users/noelbk","html_url":"https://github.com/noelbk","followers_url":"https://api.github.com/users/noelbk/followers","following_url":"https://api.github.com/users/noelbk/following{/other_user}","gists_url":"https://api.github.com/users/noelbk/gists{/gist_id}","starred_url":"https://api.github.com/users/noelbk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/noelbk/subscriptions","organizations_url":"https://api.github.com/users/noelbk/orgs","repos_url":"https://api.github.com/users/noelbk/repos","events_url":"https://api.github.com/users/noelbk/events{/privacy}","received_events_url":"https://api.github.com/users/noelbk/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-06-17T03:22:58Z","updated_at":"2018-06-17T08:50:23Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/MOcov/MOcov/pulls/14","html_url":"https://github.com/MOcov/MOcov/pull/14","diff_url":"https://github.com/MOcov/MOcov/pull/14.diff","patch_url":"https://github.com/MOcov/MOcov/pull/14.patch"},"body":"Hi and thanks for MOcov!  We'll be using it to unit test our octave project.\r\n\r\nI ran into two issues:\r\n\r\n1, we have a lot of multiline commands that the 'file' mode can't parse correctly so it injects syntax errors.  I fixed that in a pretty simple way by putting the `mocov_line_covered()` call after lines ending in ';'.  That worked quite well with our code.\r\n\r\n2. We have a lot of code directories, so I made MOcov accept multiple root_dirs\r\n\r\nAre these changes OK with you?  I still have to update the docs and unit tests.  Do you know what this unit test error means?\r\n\r\n```\r\nfailure: `    aProp;` line is wrongly classified as executable\r\n  assert:94 (/usr/share/octave/4.0.0/m/testfun/assert.m)\r\n  test_MOcovMFile_recognizes_classdef_syntax>test_properties_line_not_executable:119 (/home/noel/projects/phil/prolira/algoalpha/MOcov/tests/test_MOcovMFile_recognizes_classdef_syntax.m)\r\n\r\nfailure: Problems when running the decoated file: /tmp/AClassNNJVCUMQJOJWGIICNHHM.m - please check for syntax errors.\r\n  assert:94 (/usr/share/octave/4.0.0/m/testfun/assert.m)\r\n  test_MOcovMFile_recognizes_classdef_syntax>test_generate_valid_file:155 (/home/noel/projects/phil/prolira/algoalpha/MOcov/tests/test_MOcovMFile_recognizes_classdef_syntax.m)\r\n\r\n```\r\n\r\nThanks again for making this open source!   Nice coding style too."},"comment":{"url":"https://api.github.com/repos/MOcov/MOcov/issues/comments/397864281","html_url":"https://github.com/MOcov/MOcov/pull/14#issuecomment-397864281","issue_url":"https://api.github.com/repos/MOcov/MOcov/issues/14","id":397864281,"node_id":"MDEyOklzc3VlQ29tbWVudDM5Nzg2NDI4MQ==","user":{"login":"nno","id":1453927,"node_id":"MDQ6VXNlcjE0NTM5Mjc=","avatar_url":"https://avatars1.githubusercontent.com/u/1453927?v=4","gravatar_id":"","url":"https://api.github.com/users/nno","html_url":"https://github.com/nno","followers_url":"https://api.github.com/users/nno/followers","following_url":"https://api.github.com/users/nno/following{/other_user}","gists_url":"https://api.github.com/users/nno/gists{/gist_id}","starred_url":"https://api.github.com/users/nno/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nno/subscriptions","organizations_url":"https://api.github.com/users/nno/orgs","repos_url":"https://api.github.com/users/nno/repos","events_url":"https://api.github.com/users/nno/events{/privacy}","received_events_url":"https://api.github.com/users/nno/received_events","type":"User","site_admin":false},"created_at":"2018-06-17T08:50:23Z","updated_at":"2018-06-17T08:50:23Z","author_association":"CONTRIBUTOR","body":"Hello @noelbk,\r\n\r\nthanks for the Pull Request, much appreciated. I would definitely support the changes that you propose, under the condition that they are properly documented and tested, and that the test suite passes. So for now I will consider your PR as work in progress, and not yet as ready-to-merge-into-master.\r\n\r\nThe first error is indicating that with your changes the line ``aProp;``, when inside a class definition\r\n```\r\nclassdef foo < handle\r\n  properties\r\n    aProp;\r\n  end\r\n[...]\r\n```\r\nis considered by the parser as executable, and thus should have a call to ``mocov_line_covered`` added. It should not be considered executable; it seems your new code does not take such classdef cases correctly into account. \r\n\r\nThe second error is a bit harder to understand because the error is caught in a try-catch body. The error indicates that evaluating a string with Matlab code raised an exception. To debug this you start with temporarily removing the try-catch statement in ``test_generate_valid_file``. \r\n\r\nPlease let me know if you run into issues, or when you have updated the code with documentation and tests. \r\n\r\nBest,\r\nNick"}}
{"actor":{"display_login":"nno"}}