Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Apply SQL highlighting even if string starts with parenthesis #385

Open
macu opened this issue Mar 5, 2020 · 8 comments
Open

Apply SQL highlighting even if string starts with parenthesis #385

macu opened this issue Mar 5, 2020 · 8 comments

Comments

@macu
Copy link

macu commented Mar 5, 2020

Summary

When an SQL string starts with an open parenthesis, as used when doing UNION with LIMIT or ORDER BY applied to the individual SELECTs, the SQL highlighting doesn't apply.

Ideally the highlighter would detect and highlight the SQL even if the string begins with (SELECT

@KapitanOczywisty
Copy link
Contributor

KapitanOczywisty commented Mar 5, 2020

SQL handling is a bit wonky atm, false-positive could mess-up colors further. We need to think if there is no collision with something other than queries.

However you can use heredoc to do same thing:

$sql = <<<SQL
(SELECT * FROM `foo`)
UNION
(SELECT * FROM `bar`)
SQL;

It's not that compact, but such queries are multi-line anyway.

Edit:
This is working change if you want to try: master...KapitanOczywisty:php-sql-1

@macu
Copy link
Author

macu commented Mar 5, 2020

Was not aware of that option, thank you.

@sogerc1
Copy link

sogerc1 commented Dec 17, 2021

The problem with heredocs is that they mess up your indentation:
Screenshot_2021-12-17_12-30-46

@KapitanOczywisty
Copy link
Contributor

@sogerc1 From PHP 7.3 not anymore: https://wiki.php.net/rfc/flexible_heredoc_nowdoc_syntaxes

These are now allowed:

if(true){
    $sql = <<<SQL
        (SELECT * FROM `foo`)
        UNION
        (SELECT * FROM `bar`)
    SQL;
} else {
    $sql = <<<SQL
    (SELECT * FROM `foo`)
    UNION
    (SELECT * FROM `bar`)
    SQL;
}

@sogerc1
Copy link

sogerc1 commented Dec 17, 2021

@KapitanOczywisty nice, hopefully my servers will also have php 7.3 someday. Not very easy to upgrade a running project's servers especially in small companies.

@KapitanOczywisty
Copy link
Contributor

@sogerc1 longer you wait, you'll have bigger problems with migration. This might help with preparing project files: https://github.com/PHPCompatibility/PHPCompatibility

@keevan
Copy link

keevan commented Feb 5, 2022

SQL handling is a bit wonky atm, false-positive could mess-up colors further. We need to think if there is no collision with something other than queries.

However you can use heredoc to do same thing:

$sql = <<<SQL
(SELECT * FROM `foo`)
UNION
(SELECT * FROM `bar`)
SQL;

It's not that compact, but such queries are multi-line anyway.

Edit: This is working change if you want to try: master...KapitanOczywisty:php-sql-1

This is a cool suggestion. I wonder what's preventing you from adding this master...KapitanOczywisty:php-sql-1 as a PR? It does seem to work.

Also unfortunately usage of heredoc for SQL highlighting in some codebases I use have a PHPCS rule against it. I'd prefer to use a simple string opener (quote, double-quote) where possible and I believe I wouldn't be the only one feeling this way.

Use of heredoc and nowdoc syntax ("<<<") is not allowed; use standard strings or inline HTML instead
(Squiz.PHP.Heredoc.NotAllowed)

@KapitanOczywisty
Copy link
Contributor

KapitanOczywisty commented Feb 6, 2022

Also unfortunately usage of heredoc for SQL highlighting in some codebases I use have a PHPCS rule against it.

Heredocs are valid and proper feature of PHP, fact that someone don't want to use them is not sufficient to introduce another alternative way to mark SQL queries (pointing at -- sql). Also this shiff was introduced wayyy back when heredocs couldn't handle indentations, nowadays this is not an issue.

This is a cool suggestion. I wonder what's preventing you from adding this master...KapitanOczywisty:php-sql-1 as a PR? It does seem to work.

It wasn't tested and properly thought through

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

No branches or pull requests

4 participants