yiisoft / yii2-debug

Debug Extension for Yii 2
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
202 stars 150 forks source link

Enhancement of SQL output #161

Open dynasource opened 7 years ago

dynasource commented 7 years ago

this deserves attention regarding readability. Having 1 line of SQL is not really practical.

samdark commented 7 years ago

Yes. Automatic formatting could be useful.

dynasource commented 7 years ago

a similar usecase can be found for the display of database exceptions pages. Where do we want to put the formatting logic?

dynasource commented 7 years ago

perhabs formatting by JS with an Asset

cebe commented 7 years ago

I'd prefer to not introduce too much javascript which could interferre with the loaded page so a PHP solution like it is used in apidoc extension can be used: https://github.com/yiisoft/yii2-apidoc/commit/1b5813c4960d9a0a9d9f0d29912c5d578b2da9de

dynasource commented 7 years ago

on the contrary, we will have more places in which SQL formatting could be appropiate. This will pollute the core while JS could be unobtrusive.

What about adding class="mysql" to those containers. Then the code would not have to interfere?

samdark commented 7 years ago

How JS could be unobtrusive? It pullutes dependencies and we have a plan to move all JS dependencies out of the core...

dynasource commented 7 years ago
samdark commented 7 years ago

If it's supplied by debug then it should be fine.

dynasource commented 7 years ago

how should we integrate it with \yii\db\Exception?https://github.com/yiisoft/yii2/blob/master/framework/db/Schema.php#L635

isnt this a clear case to introduce an Event to the Exception architecture?

Then we can handle all inside the debug extension.

samdark commented 7 years ago

isnt this a clear case to introduce an Event to the Exception architecture?

Isn't it too late to process events when there's an uncaught exception?

samdark commented 7 years ago

I'd introduce $sql property in DB Exception class so it could be obtained.

dynasource commented 7 years ago

just by including googles prettifier and some PHP regex

<script src="https://cdn.rawgit.com/google/code-prettify/master/loader/run_prettify.js"></script>

we can get this: capture

Do we like this?

alexraputa commented 7 years ago

👎 It might look better.

samdark commented 7 years ago

@alexraputa any proposals?

rob006 commented 7 years ago

Just compare this to real formatters: http://www.dpriver.com/products/sqlpp/sqlexamples2.php

alexraputa commented 7 years ago

@rob006, 👍 it looks much better.

dynasource commented 7 years ago

Just compare this to real formatters: http://www.dpriver.com/products/sqlpp/sqlexamples2.php

this is a paid one? http://www.dpriver.com/buynow.php

It would be nice to have use an open source library for this. This can be time consuming to create yourself.

rob006 commented 7 years ago

First result from google: https://github.com/jdorn/sql-formatter

dynasource commented 7 years ago

jdorn looks nice indeed. A nice example at http://jdorn.github.io/sql-formatter/

jdorn

Unless people have better proposals, I'll integrate that one.

dynasource commented 7 years ago

I've looked into the Sql formatters. It seems that 'jdorn/sql-formatter' has the most stars on github and from what I have seen, also the most readable output.

It has one disadvantage though. Readability goes at cost of the length of the page. Should I integrate a 'onclick' prettify toggle?

samdark commented 7 years ago

Length doesn't matter in debug mode.

dynasource commented 7 years ago

well, having many of these page filling queries is quite obtrusive. People have to expect another userexperience on the DB debug panel

samdark commented 7 years ago

Won't matter much.

dynasource commented 7 years ago

it matters on a 30 inch ;)

cebe commented 7 years ago

It has one disadvantage though. Readability goes at cost of the length of the page. Should I integrate a 'onclick' prettify toggle?

That would be good to have otherwise scrolling through a lot of querys is tedious when they are complex.

samdark commented 7 years ago

The toggle should be sticky then. Its state should survive page reloads.

dynasource commented 7 years ago

status update: jdorn/sql-formatter does not to seem to be stable and gives errors with certain SQL's. This is not desired behavior for inclusion.

samdark commented 7 years ago

@dynasource worth reporting to jdorn/sql-formatter issues...

dynasource commented 7 years ago

I'm questioning to keep it simple instead of having a library to fix it. On the other hand, perhabs we should adopt flexibility and make the developer to decide which sql formatter he should use

samdark commented 7 years ago

Well, reporting it is a good idea anyways.

samdark commented 7 years ago

Some degree of flexibility is good.