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

Refactored the doctrine_pretty_query filter to support "highlight only" mode #477

Merged
merged 4 commits into from
Nov 4, 2015

Conversation

javiereguiluz
Copy link
Contributor

This fixes #469

@stof
Copy link
Member

stof commented Oct 15, 2015

@javiereguiluz please clean your branch from all these merge commits. It looks like your local master branch diverged form the upstream one.

@stof
Copy link
Member

stof commented Oct 15, 2015

ok, diff got lost now

@javiereguiluz
Copy link
Contributor Author

The PR is now ready to be reviewed. Thanks.

} else {
$html = \SqlFormatter::format($sql);
}

$html = preg_replace('/<pre class="(.*)">([^"]*+)<\/pre>/Us', '<div class="\1"><pre>\2</pre></div>', $html);
Copy link
Member

Choose a reason for hiding this comment

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

this could be simplified, by changing SqlFormatter::$use_pre instead

Copy link
Member

Choose a reason for hiding this comment

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

and when highlighting only, should we really return a <pre> tag ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't use $use_pre because is a "global state" and we need to output different things depending whther we highlight or not. I've just updated the code.

guilhermeblanco added a commit that referenced this pull request Nov 4, 2015
Refactored the doctrine_pretty_query filter to support "highlight only" mode
@guilhermeblanco guilhermeblanco merged commit ba11cc4 into doctrine:master Nov 4, 2015
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.

Merge the "doctrine_pretty_query" and "doctrine_highlight_sql" Twig filters
3 participants