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

Embedded SQL-Syntax only works for inline SQL #87

Open
screeny05 opened this issue Jun 29, 2015 · 22 comments
Open

Embedded SQL-Syntax only works for inline SQL #87

screeny05 opened this issue Jun 29, 2015 · 22 comments

Comments

@screeny05
Copy link

Used Atom Version: 1.0
Expected behaviour: Correct highlighting of SQL within PHP-Strings
Has this ever worked: No

Highlighting of SQL within strings is a very useful feature, but there's a little problem:

the language-php package only seems to highlight them if the beginning of the sql-statement lies on the same line as the opening of the string with quotation marks.

image
(Notice, that Lines 6-8 are all green)

@50Wliu 50Wliu added the bug label Jun 29, 2015
@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

Atom uses textmate like grammar, if main principles of grammar is same then this one is impossible.
Textmate grammar limited to 1 rule per line, so it not possible to match few lines with 1 regex, here is info about that behavior Textmate Grammar

That problem require a multiline matching of text, because it is not possible otherwise distinguish SQL string from regular string basing only on quote.

Someone can fill bug/question about grammar to core devs?

@thisispiers
Copy link
Contributor

any updates? an issue where the string is incorrectly ended occurs with concatenated, embedded sql and it screws up the highlighting of the rest of the file

@zr9
Copy link
Contributor

zr9 commented Feb 5, 2016

@thisispiers You probably talking about other one, could you post test example which code broke highlight? That one is related only to case when you not get correct highlight if SQL is not single line.

@Arcanemagus
Copy link

This is the same issue as #52, the switch to SQL gets broken with any break in the string. (Including, apparently, a newline.)

@50Wliu
Copy link
Contributor

50Wliu commented Apr 22, 2016

@Arcanemagus I'm going to keep this one separate just in case.

@zr9
Copy link
Contributor

zr9 commented Apr 22, 2016

@Arcanemagus @50Wliu
In concept of bugs this is 2 different issues, #52 can be solved. When this one almost nope. Since tokenizer not support multiline capture

@50Wliu
Copy link
Contributor

50Wliu commented Apr 22, 2016

Since tokenizer not support multiline capture

Yes it does: just use begin/end instead of match.

@zr9
Copy link
Contributor

zr9 commented Apr 22, 2016

@50Wliu Yup in global it does. Problem so being and end is match only edges, and in that case there will be no different between SQL inside quotes and Str inside quotes. For detect this case there is need proper multiline regex with checks what is between begin and end. So for example is not possible to make expression(as simple example) like /"\s*SELECT.*"/ because part .* not catch multiline.

The other way to do it, is do quotes support in grammar and all cases inside them, it works if i'm correctly recall atom and textmate grammar. But itself way take too much time and efforts and still can give false positive in that case.

The third way is to set begin and end to some uniq to SQL tokens, problem so SQL not have some special end token.

@james-radford-854
Copy link

We could match /"\s*--\s*SQL$/, which would match the following:

$foo = "-- SQL
    SELECT *
    FROM users
    WHERE foo = 'bar'
";

@zr9
Copy link
Contributor

zr9 commented Aug 9, 2016

@james-radford-854 That is a bit crazy way to invent syntax to cover highlight i'm would say. Since in that case everyone need to agree on that way and keep it, which i'm assume not happens. But yup will work in total

@PeteDevoy
Copy link

PeteDevoy commented Sep 14, 2016

I assume most people use PDO now and most people name their variables for SQL $sql, $query or $statement. So if the code could pattern match for ->exec(, ->prepare(, $sql =, etc. it might be an acceptable compromise?

̣Even if it's a bad idea because of false positives I would like to try for myself anyway. Please could someone tell me where in the file I would need to write the patterns? Disregard that, looks like there is no easy way to hack at the grammar files.

@zr9
Copy link
Contributor

zr9 commented Sep 14, 2016

@PeteDevoy It basically not hard, you need file /grammars/php.cson. And you need to use begin/end to capture multiline, end in your case probably will be /["'];/ and begin some like /$sql\s*=/. But please keep in mind it only examples, in reality you need to set them in synergy with others, because if i'm not wrong if you capture group which belongs to other then other will fall. May be wrong since used grammar a bit time ago. As reference look on string-double-quoted group, it almost close to perfect example i'm assume, since it have not only group capture but pattern include as well. And please keep in mind so your end syntax need too be a unique string, since if i'm not wrong it dumb capture, and so it captures without patterns check and then only start pattern checking.

@PeteDevoy
Copy link

PeteDevoy commented Sep 15, 2016

@zr9 Yes, thanks the tips. What you are saying sounds perfectly correct but unfortunately I do not think /grammars/php.cson in file:///~/.atom/? I believe I would have to change at source and then do a custom build. Although it might be simple technically speaking, custom building Atom sounds like a it cause headaches :(

@50Wliu
Copy link
Contributor

50Wliu commented Sep 15, 2016

@PeteDevoy apm develop language-php will clone language-php to ~/github/language-php. Then you can start atom using atom --dev. Changes made to language-php will be reflected there (just reload the window).

@zr9
Copy link
Contributor

zr9 commented Sep 15, 2016

@PeteDevoy Yup, as @50Wliu said you can use source of package, it not require rebuilt whole atom source. If you not like apm for some reason you can directly make a clone on this repository in atom packages dir, it will also work.

@50Wliu 50Wliu added the blocked label Jun 9, 2017
@adjenks
Copy link

adjenks commented Feb 19, 2019

In response to @james-radford-854 who said this:

We could match /"\s*--\s*SQL$/, which would match the following:

$foo = "-- SQL
    SELECT *
    FROM users
    WHERE foo = 'bar'
";

I don't think there's anything wrong with controlling highlighing in comments. Perhaps a docblock tag would be better though. Maybe get in touch with the phpDocumentor creators and ask for some namespace in their keywords? https://phpdoc.org/contact

Something like this for example:

/**
  * @lang SQL
  */
 $foo = "
     SELECT *
     FROM users
     WHERE foo = 'bar'
 ";

@JimmyBastos
Copy link

I'm facing the same problem. Any update on this?

@adjenks
Copy link

adjenks commented Aug 15, 2019

If we used /**@DocBlocks*/, or something similar, you could easily toggle sections of code to use different languages embedded in strings and have no issue about identifying them. Much like how we insert a language into markdown with three back-ticks and then the language.

```SQL
SELECT 'like so' FROM information_schema.tables WHERE true = true;
```

results in:

SELECT 'like so' FROM information_schema.tables WHERE true = true;

@mbomb007
Copy link

I don't think a comment is enough. I think it should work with multiline regex.

@KapitanOczywisty
Copy link
Contributor

KapitanOczywisty commented Jun 12, 2020

@mbomb007 grammar engine is limited, but it's still possible to use heredoc:

$query = <<<SQL
    SELECT `column`
    FROM `table`
SQL;

@keevan
Copy link

keevan commented Feb 5, 2022

@mbomb007 grammar engine is limited, but it's still possible to use heredoc:

$query = <<<SQL
    SELECT `column`
    FROM `table`
SQL;

Whilst it's possible to use heredoc, it might conflict with some existing coding standards on projects it's an alternative that might not work for everyone (highlighted here: #385 (comment))

Is the grammar engine issue fixed by this PR? #303 . If my understanding is correct, it's currently using TextMate's grammar engine which does not support the level of multi-line matching we need for this to work, and the conversion to using tree-sitters (https://flight-manual.atom.io/hacking-atom/sections/creating-a-grammar/) is the way forward, should be more flexible, and will allow this issue to be fixed without weird workarounds + annotations?

@blaumeise20
Copy link

Please update this! It's really annoying having to write the whole query in one line just to get syntax highlighting.

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

No branches or pull requests