Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] made the pretty printer a little bit prettier #39

Closed
wants to merge 1 commit into from

Conversation

pscheit
Copy link

@pscheit pscheit commented Oct 18, 2012

I added a few more tests for the printer. The switch looked really strange for me.
If this breaks something with the Zend Conventions I'd like to create a new Pretty Printer which is more closely to PSR-2 then.

added more tests for the pretty printer
fixed switchcasebreak
fixed closure bracketing in body and around
@@ -422,7 +422,7 @@ public function pExpr_Closure(PHPParser_Node_Expr_Closure $node) {
return ($node->static ? 'static ' : '')
. 'function ' . ($node->byRef ? '&' : '')
. '(' . $this->pCommaSeparated($node->params) . ')'
. (!empty($node->uses) ? ' use(' . $this->pCommaSeparated($node->uses) . ')': '')
. (!empty($node->uses) ? ' use (' . $this->pCommaSeparated($node->uses) . ')': '')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't know if this is against Zend Coding standard

nikic added a commit that referenced this pull request Oct 31, 2012
Previously the pretty printer added unnecessary and odd-looking parentheses
when several operators with the same precedence were chained:

    'a' . 'b' . 'c' . 'd' . 'e'
    // was printed as
    'a' . ('b' . ('c' . ('d' . 'e')))

Another issue reported as part of #39 was that assignments inside closures
were wrapped in parentheses:

    function() {
        $a = $b;
    }
    // was printed as
    function() {
        ($a = $b);
    }

This was caused by the automatic precedence handling, which just regarded
the closure as an ordinal nested expression.

With the new system the $predenceMap of PrettyPrinterAbstract contains both
precedence and associativity and there is a new method pPrec() which prints
a node taking precedence and associativity into account.

For simpler usage there are additional function pInfixOp(), pPrefixOp() and
pPostfixOp().

Prints not going through pPrec() do not have any precedence handling (fixing
the closure issue).
nikic added a commit that referenced this pull request Oct 31, 2012
The switch cases were not indented and fall-through cases had an
unnecessary additional newline.

Patch by @pscheit (PR #39).
@nikic
Copy link
Owner

nikic commented Oct 31, 2012

Sorry for the long response time. I fixed the closure issue as part of implementing associativity handling (which will make the printing of operator chains prettier) and also committed the switch formatting changes you did.

I didn't do much testing of the pretty printer output yet, so there are probably a few more ugly places.

@nikic nikic closed this Oct 31, 2012
@ghost
Copy link

ghost commented Jan 5, 2013

I would love to see something that does PSR-2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants