Add SQL block highlighting support

[aceArt-GmbH]

Fixes #831

Before:

After:

I am not familiar with vscode extensions, please review carefully

[rgrunber]

Nice. I definitely like that this part of the grammar can be injected into the necessary scope.

[aceArt-GmbH]

Please review the regex

@fbricon is this solution in scope of this extension?

[fbricon]

It's pretty nice indeed (when it works). I like the fact the solution is not too invasive, so the current java grammar can still be picked up by upstream vscode, without being impacted.

Things that don't work:

• no reason why single line strings should not be supported
• sql comment on single line string breaks string highlighting
• sql comment on single line string breaks following comments highlighting

public class SQLStrings {

  String REGULAR_SQL = "SELECT * FROM TABLE";

  String SQL2 = /* sql with more comment doesn't work */ 
      """
        SELECT * FROM TABLE
      """;

  // sql breaks single line highlighting:
  String ONE_LINE_SQL = /* sql */ "SELECT * FROM TABLE";
  
  //previous statement breaks following comments
  //copied from https://github.com/0x00000001A/es6-string-html/issues/10#issuecomment-412294344
  String NICE_SQL = /* sql */
    """
    -- Test comment
	-- Some other comment about.someQueue.else

	SELECT
  queue.id,
  queue.timestamp,
  queue.someInt,
  queue.column
FROM queue

/* Block comment */
LEFT JOIN queue AS queue_blocked ON (
  queue_blocked.id_blocked IS NOT NULL
  AND

  /* 
    Block comment 
    Block comment 
  */
  queue_blocked.type IN ('someQueue')

  AND
  queue_blocked.id_blocked = queue.id
  AND
  queue_blocked.status NOT IN ('success', 'failed')
)
WHERE
  -- Comment about noQueue
  queue_blocked.id IS NULL

  AND
  queue.type IN ('someQueue')
  AND
  queue.status NOT IN ('success', 'failed')
  AND
  (
    (
      queue.retry_count < queue.retry_max

      -- Avoid running this too often (too soon)
      AND
      UNIX_TIMESTAMP(TIMESTAMP(NOW())) - UNIX_TIMESTAMP(TIMESTAMP(REPLACE(REPLACE(queue.updated_timestamp, 'T', ' '), 'Z', ''))) >= queue.retry_min_sec
    )
    OR
    queue.call_count = 0
  )

  -- Slow subquery replaced by LEFT JOIN + WHERE filtering trick.
  /*AND
  NOT EXISTS (
    SELECT
      1 AS it_exists
    FROM queue AS queue_A
    WHERE
      -- Optimization, doesn't affect results.
      queue_A.type IN ('someQueue')

      -- Optimization, doesn't affect results.
      AND
      queue_A.status NOT IN ('success', 'failed')

      -- Optimization, doesn't affect results.
      -- This optimization made the whole query 5 (five) times faster.
      AND
      (
        queue_A.group_id = queue.group_id
        OR
        (
          queue_A.group_id IS NULL
          AND
          queue.group_id IS NULL
        )
      )

      -- The mandatory condition
      AND
      queue_A.id_blocked = queue.id
    LIMIT 1
  )*/


  AND
  (
    queue.id_blocked_by IS NULL
    OR
    NOT EXISTS(
      SELECT
        1 AS it_exists
      FROM queue AS queue_C
      WHERE
        (
          queue_C.group_id = queue.group_id
          OR
          (
            queue_C.group_id IS NULL
            AND
            queue.group_id IS NULL
          )
        )
        AND
        queue_C.id = queue.id_blocked_by
      LIMIT 1
    )
  )
  AND
  (
    queue.id NOT IN (${this.runningQueueIDs().length ? this.runningQueueIDs().join(",") : 0})
    OR
    (
      queue.mark_for_death IN ('kill' ,'stop_retrying', 'kill_and_stop_retrying')
      AND
      queue.process_info_json IS NOT NULL
      AND
      JSON_EXTRACT(queue.process_info_json, "$.property") = ${escape(arch())}
      AND
      JSON_EXTRACT(queue.process_info_json, "$.pid") = ${escape(strSomeJavaScriptStringVar)}
    )
  )
ORDER BY
  queue.id ASC
LIMIT ${this._objQueueConfig.max_workers - this.runningQueueIDs().length}
    """;
}

One concern I have though, is it makes it specific to VS Code. Given that IntelliJ has a similar support with language injection comments, using the // language=sql format, I think it'd be better to reuse that:

[aceArt-GmbH]

Thanks @fbricon for testing. I updated the code accordingly.

Having other words in a comment breaking highlighting is somewhat intentional. We don't want to highlight any string, just because a comment mentions "sql".
But I think now with language=sql it probably won't happen on accident.

[rgrunber]

I think this is ready to merge. I just had some questions about whether certain parts can be removed (ie. redundant/not working) to make this easier to maintain.

[rgrunber]

My understanding of L:source.java -comment -string is it's supposed to inject into the source.java scope but exclude comment.* & string.* scopes. I'm not sure why we would avoid injecting into the string.* scopes because that's definitely what we want. We want to highlight text blocks that match language=sql pattern.

As for comment.* scopes, from the below example, that seems like a comment block, but the sql syntax still is activated.

I guess my only point is if it's safe to make this "injectionSelector": "L:source.java", without changing any existing functionality, I would do that.

[aceArt-GmbH]

There seems to be no real multi line support available.
In the current state of this PR, there are 2 separate regex selectors.
This means, adding a /* language=sql */ anywhere in the file, will destroy color highlighting until the second regex matches a ".
This is really bad and also explains the comment situation in your example.
"es6-string-html" has currently the same fate :/
https://macromates.com/manual/en/regular_expressions mentions ?m should add multi line support, but I think VSCode does not implement it.

[rgrunber]

Alright, I guess we can live with that. Can you remove -comment -string as well as fileTypes without it breaking anything ? If so, feel free to do so, and I think we can merge this. If there's something we lose because of it, we can keep it. I just want to ensure everything added is needed (for the sake of easier maintenance).

[aceArt-GmbH]

With -comment -string the following two examples look like strings/comments.
Without it, the variable would look like a string/comment, but the value would have color highlighting, which I find odd

String x = """
  String ONE_LINE_SQL = // language=sql
  "SELECT * FROM TABLE";
""";

/*
String ONE_LINE_SQL = // language=sql
"SELECT * FROM TABLE";
*/

[rgrunber]

Ok, so then it does help under such cases. The example I had a few comments above, was just one edge case not handled by the exclusions.

[rgrunber]

There's definitely some oddities, but given that everything is guarded off by "language=sql" and not leaking into regular usage, we can go ahead with this and propose additional fixes as needed.