Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Parse docblock indented with tabs #3273

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

mtorromeo commented Dec 21, 2012

This fixes the regular expression used to clean-up the docComments.
The tab escape (\t) was not parsed correctly because it was specified inside a single quoted string.

Additionally the DocBlockScanner did not catch tabs as whitespace tokens.

This fixes #2445

mtorromeo added some commits Dec 21, 2012

Parse docblock indented with tabs
This fixes the regular expression used to clean-up the docComments.
The tab escape (\t) was not parsed correctly because it was specified
inside a single quoted string.

Additionally the cleaned-up doc comment ($this->cleanDocComment) was
used by the DocBlockScanner.

This fixes #2445
Fixed DOCBLOCK_WHITESPACE token to detect tabs
This definetely fixes #2445 and passes all unit tests.
New unit tests for docblocks indented with tabs that
fail to be parsed without this fix.
Contributor

mtorromeo commented Dec 21, 2012

I just noticed that there is a test that is not passed and that is the code style checker:
https://travis-ci.org/zendframework/zf2/jobs/3778614

I think this is a false positive and actually an exception to the rule since I specifically added a class to the unit tests with tabs indentation (that is the whole point of the test).

@Maks3w Maks3w commented on an outdated diff Dec 22, 2012

library/Zend/Code/Scanner/DocBlockScanner.php
@@ -269,7 +269,7 @@ protected function tokenize()
goto TOKENIZER_TOP;
}
- if ($currentChar === ' ') {
+ if ($currentChar === ' ' or $currentChar === "\t") {
@Maks3w

Maks3w Dec 22, 2012

Member

Use || instead of or

Contributor

mtorromeo commented Jan 2, 2013

Is this good for merge now?

weierophinney added a commit that referenced this pull request Jan 3, 2013

[#3273] Added PHP-CS-Fixer rules
- Omit test file known to have issues (as they are required for tests)

weierophinney added a commit that referenced this pull request Jan 3, 2013

Owner

weierophinney commented Jan 3, 2013

Looks good. I added .php_cs files in the project root and tests directory to exclude the one file when running CS checks.

@ghost ghost assigned weierophinney Jan 3, 2013

weierophinney added a commit to zendframework/zend-i18n-resources that referenced this pull request May 28, 2015

[zendframework/zendframework#3273] Added PHP-CS-Fixer rules
- Omit test file known to have issues (as they are required for tests)

weierophinney added a commit to zendframework/zend-i18n-resources that referenced this pull request May 28, 2015

weierophinney added a commit to zendframework/zend-i18n-resources that referenced this pull request May 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment