{"action":"created","issue":{"url":"https://api.github.com/repos/wp-cli/php-cli-tools/issues/78","labels_url":"https://api.github.com/repos/wp-cli/php-cli-tools/issues/78/labels{/name}","comments_url":"https://api.github.com/repos/wp-cli/php-cli-tools/issues/78/comments","events_url":"https://api.github.com/repos/wp-cli/php-cli-tools/issues/78/events","html_url":"https://github.com/wp-cli/php-cli-tools/pull/78","id":58422238,"number":78,"title":"Fixed the 'No arg value warning' when an option is paased with no value ...","user":{"login":"rantonmattei","id":1832073,"avatar_url":"https://avatars.githubusercontent.com/u/1832073?v=3","gravatar_id":"","url":"https://api.github.com/users/rantonmattei","html_url":"https://github.com/rantonmattei","followers_url":"https://api.github.com/users/rantonmattei/followers","following_url":"https://api.github.com/users/rantonmattei/following{/other_user}","gists_url":"https://api.github.com/users/rantonmattei/gists{/gist_id}","starred_url":"https://api.github.com/users/rantonmattei/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rantonmattei/subscriptions","organizations_url":"https://api.github.com/users/rantonmattei/orgs","repos_url":"https://api.github.com/users/rantonmattei/repos","events_url":"https://api.github.com/users/rantonmattei/events{/privacy}","received_events_url":"https://api.github.com/users/rantonmattei/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-02-20T22:56:02Z","updated_at":"2015-02-25T04:08:22Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/wp-cli/php-cli-tools/pulls/78","html_url":"https://github.com/wp-cli/php-cli-tools/pull/78","diff_url":"https://github.com/wp-cli/php-cli-tools/pull/78.diff","patch_url":"https://github.com/wp-cli/php-cli-tools/pull/78.patch"},"body":"...as the last arg.\r\n\r\nThis is a fix for https://github.com/wp-cli/php-cli-tools/issues/77\r\nThis code takes care of the case where the last option passed has no value. The warning will be thrown as well.\r\n\r\nliterally it is a simplified version of:\r\n```php\r\nif ($this->_lexer->end() || (!$this->_lexer->end() && !$this->_lexer->peek->isValue)) {\r\n    //...\r\n}\r\n```\r\n"},"comment":{"url":"https://api.github.com/repos/wp-cli/php-cli-tools/issues/comments/75902430","html_url":"https://github.com/wp-cli/php-cli-tools/pull/78#issuecomment-75902430","issue_url":"https://api.github.com/repos/wp-cli/php-cli-tools/issues/78","id":75902430,"user":{"login":"rantonmattei","id":1832073,"avatar_url":"https://avatars.githubusercontent.com/u/1832073?v=3","gravatar_id":"","url":"https://api.github.com/users/rantonmattei","html_url":"https://github.com/rantonmattei","followers_url":"https://api.github.com/users/rantonmattei/followers","following_url":"https://api.github.com/users/rantonmattei/following{/other_user}","gists_url":"https://api.github.com/users/rantonmattei/gists{/gist_id}","starred_url":"https://api.github.com/users/rantonmattei/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rantonmattei/subscriptions","organizations_url":"https://api.github.com/users/rantonmattei/orgs","repos_url":"https://api.github.com/users/rantonmattei/repos","events_url":"https://api.github.com/users/rantonmattei/events{/privacy}","received_events_url":"https://api.github.com/users/rantonmattei/received_events","type":"User","site_admin":false},"created_at":"2015-02-25T04:08:22Z","updated_at":"2015-02-25T04:08:22Z","body":"@danielbachhuber, I added the tests. Since the `_parseOption ` method required a few step before we can call it, I added tests for those steps. (i.e. `__constructor()`, `addFlags()`, `addOptions()` methods)\r\nAs I was writing tests I ran into a couple of issues: \r\n - the first one was mocking the `$_SERVER` global, which you cannot do but PhpUnit does give you the ability to back it up and then clear it so you can use it for testing. See https://phpunit.de/manual/current/en/appendixes.annotations.html#appendixes.annotations.backupGlobals\r\n - tests were failing after I added dataProviders. This one took me a while to track down, and I finally found this evil line of code :imp: : https://github.com/wp-cli/php-cli-tools/blob/master/lib/cli/arguments/Lexer.php#L71\r\n\r\n It is using a static variable as a flag in the `Lexer::rewind()` method, so the first time you parse, a `foreach` is called and triggers the `Lexer::rewind()` just fine but the second time the `Lexer::rewind()` method does not do anything because that var has been set to `false`. And since Phunit was looping through dataProviders, it was calling the `parse()` and `_parseOption()` methods many times (and so was the `Lexer::rewind()` method). It looks like the `parse()` method is meant to be called once within an execution context. Therefore the iterator was never rewound, I guess.\r\n\r\nSo, for the latter issue I had to tweak that a bit and I made that flag a static class variable instead so I could reset the flag after parsing is done. After that I was able to test without any issues.\r\n\r\nThis static var in the `Lexer::rewind()` method seems very hacky to me. The `Lexer` class would deserve some refactoring especially the `Iterator` implementation. The `Arguments` class could be simplified as well. But I don't really have the bandwidth for that refactor at the moment.\r\n\r\nI hope this is clear. Let me know if you need any further explanation. I have another fix for the default option that does not populate when it is empty, but there is a dependency on that PR.\r\n\r\nPlease review.\r\nThanks"}}