atom / language-php

PHP package for Atom
Other
119 stars 118 forks source link

Apply SQL highlighting even if string starts with parenthesis #385

Open macu opened 4 years ago

macu commented 4 years ago

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 commented 4 years ago

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: https://github.com/atom/language-php/compare/master...KapitanOczywisty:php-sql-1

macu commented 4 years ago

Was not aware of that option, thank you.

sogerc1 commented 2 years ago

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

KapitanOczywisty commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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

keevan commented 2 years ago

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 https://github.com/atom/language-php/compare/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 commented 2 years ago

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