Formatter inconsistencies

[bigfoot90]

I'm rewriting Formatter's tests, and during my PR #120 I've noticed some inconsistencies and I would discuss them with you.

1 Create procedure (Done in #152)
https://github.com/bigfoot90/sql-parser/blob/d957633d2259f293b77affb3f8e223f4ed7cf7f2/tests/Utils/FormatterTest.php#L352-L357

CREATE
PROCEDURE test_procedure() BEGIN
FROM
    tbl
SELECT
    *; END

• CREATE PROCEDURE should be on the same line (like SELECT DISTINCT already does)
• BEGIN and END should go to a new line

2 Insert into (Done this too)
https://github.com/bigfoot90/sql-parser/blob/d957633d2259f293b77affb3f8e223f4ed7cf7f2/tests/Utils/FormatterTest.php#L376-L379

INSERT
INTO
    foo
VALUES(0, 0, 0),(1, 1, 1)

• INSERT INTO should be on the same line (like SELECT DISTINCT)
• Put a blank space after VALUES keyword?
• The values (0, 0, 0) and (1, 1, 1) should go to their own rows (like the other query parts does) if the option parts_newline is true (the default).

I haven't ended with #120 yet, I will add here more questions about formatting as soon I will find them.

[nijel]

Yes, the text formatter could clearly do better here.

[bigfoot90]

3 Non printable characters
Non printable characters are escaped only when formatting from CLI.
The Formatter's output should be the same regardless of the format used.
https://github.com/bigfoot90/sql-parser/blob/abbf4e786703b63a4191ae831ed1157267891663/tests/Utils/FormatterTest.php#L403-L414

[nijel]

I'm not really sure what is best approach here. It needs to be escaped for console to avoid injection (see #84).

For HTML, we clearly need to escape HTML special chars, what we do (there was also bug in this, see #83), but the question is what to do with chars which are not significant in HTML, but will not be properly rendered.

[bigfoot90]

I see two ways to threat this correctly:

Option 1
Strip them all because they are not visible by default (except from cli)

Option 2
Make them visible to all formats, not only to cli.
Since the input query is the same, the output should be too.
All non printable characters can be converted to their correspective hexadecimal code, and concatenate to the rest of the string if any.

Proposed examples:

$ q=`echo -e "select '\x1b'"`
$ ./bin/highlight-query --query "$q"
SELECT 0x1b

$ q=`echo -e "select 'text\x1b-inj'"`
$ ./bin/highlight-query --query "$q"
SELECT concat('text', 0x1b, '-inj')

PhpMyAdmin dosen't use CONCAT(), it converts the entire string into hexadecimal representation if any non printable character is found, maybe we should do the same.

Edit:
In any case
From my point of view, the current behaior is vrong as the query is changed.
"\x1b" is not the same as "\\x1b"
"\x1b" is the same as 0x1b

[nijel]

This should be just pretty printing the query, I don't think we should do any changes to it like this...

[bigfoot90]

Yep, but if run this same query on cli and html you end with two different queries.

[bigfoot90]

Closing this.