Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(sql) 'foreign key' and 'primary key' do not highlight 'key'. #4057

Open
htozaki opened this issue May 24, 2024 · 4 comments
Open

(sql) 'foreign key' and 'primary key' do not highlight 'key'. #4057

htozaki opened this issue May 24, 2024 · 4 comments
Labels
bug good first issue Should be easier for first time contributors help welcome Could use help from community language

Comments

@htozaki
Copy link

htozaki commented May 24, 2024 •
edited
Loading

Describe the issue
When I write foreign key or primary key is sql language, primary or foreign is highlighted, however key is not.

Which language seems to have the issue?
sql.

Are you using highlight or highlightAuto?

highlightAll().

Sample Code to Reproduce

<!DOCTYPE html>
<html lang="en">
<head>
<title>test</title>
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/agate.min.css">
<script src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/highlight.min.js"></script>
<script>
hljs.highlightAll();
</script>
</head>
<body>
<pre>
<code class="sql">
create table Book (
    bookId      int,
    publisherId int,
    primary key(bookId),
    foreign key(PublisherId) references Publisher(id)
);
</code>
</pre>
</body>
</html>

Expected behavior
'key' is also highlighted.

Additional context
Even though I add the following to my script to force the word 'key' as keyword,

const custom_keywords = ['key'];
hljs.getLanguage('sql').keywords.keyword.push(...custom_keywords);

key won't be highlighted. (However this addition will now highlights foreign key or foreign&nbsp;key primary&#8288; key etc when the separator is not a single space.)

I think the regexp using in COMBO does not recognize space as a part of keyword, that is the problem.
Supposing the intention of this code is to treat foreign key as one keyword.
In my script, changing the regexp so as to include space character

hljs.getLanguage('sql').contains[0].keywords.$pattern = /\b[\w\. ]+/;

solve the problem.

However I have not understand yet why create table is working as intended, so I am not sure this is correct fix.

Also it does not highlights when I write foreign key using 2 spaces etc, unless I added the key in keyword.

Though I have not understand the whole code yet, my suggestion would be the regexp at

highlight.js/src/languages/sql.js

Line 660 in 15d3b62

$pattern: /[\w\.]+/,

should be changed to recognize space as a part of keyword. (Also it need to add key in reserved word list

highlight.js/src/languages/sql.js

Line 99 in 15d3b62

const RESERVED_WORDS = [

in case there are 2 or more whitespaces between foreign and key.)
@htozaki htozaki added bug help welcome Could use help from community language labels May 24, 2024
@joshgoebel
Copy link
Member

It's because key isn't a keyword on its own in standard SQL I don't think? The solution is to add it to the the specific list that we're using for combo highlighting.
@joshgoebel joshgoebel added the good first issue Should be easier for first time contributors label May 25, 2024
@htozaki
Copy link
Author

htozaki commented May 26, 2024 •
edited
Loading

However key part of foreign key is not highlighted.
See the SQL part of https://highlightjs.org/examples

(Note that foreign key and primary key is already in the combo list.

highlight.js/src/languages/sql.js

Lines 583 to 584 in 15d3b62

"primary key",
"foreign key",
)
@joshgoebel
Copy link
Member

I'm trying to remember what the \. is for, none of the keywords have a literal ..

I think for combos we may want somethingl ike:

 $pattern: /\w+(\s+\w+)?/,
@htozaki
Copy link
Author

htozaki commented May 27, 2024 •
edited
Loading

Aside from literal period, changing regular expression of $pattern is not enough to fix this problem.

When the text foreign key matches, parser goes to:

highlight.js/src/highlight.js

Line 214 in 4bbf361

const data = keywordData(top, word);

If the word equals to one of the keywords it returns data, however it requires exact match. So when the text is foreign key or something the returned data becomes undefined i.e. no highlights occurs there.

So we need to modify keywordData() or consider different handling with COMBOS.

Quick fix would be the following. It looks not good, furthermore I am afraid it might affect other languages.

First, change the keyword to regexp in sql.js (timezone in with timezone and without timezone need to be converted as well.)

      contains: [
        {
          begin: regex.either(...COMBOS.map((x)=>x.replaceAll(/\s+/g, "\\s+"))),
          relevance: 0,
          keywords: {
            $pattern: /\w+(?:\s+\w+)?/,
            keyword: KEYWORDS.concat(COMBOS.map((x)=>x.replaceAll(/\s+/g, "\\s+"))),
            literal: LITERALS,
            type: TYPES
          },
        },
        {
          className: "type",
          begin: regex.either(...MULTI_WORD_TYPES.map((x)=>x.replaceAll(/\s+/g, "\\s+")))
        },

Then in highligts.js

      function keywordData(mode, matchText) {
        return mode.keywords[matchText.match(/\s+/) ?
          matchText.replaceAll(/\s+/g, "\\s+") : // to the exact (not equivalent) regular expression.
          matchText];
      }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Should be easier for first time contributors help welcome Could use help from community language
2 participants
@joshgoebel @htozaki