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

Fix/issue 3972 #3989

Merged
merged 8 commits into from
Feb 25, 2024
Merged

Fix/issue 3972 #3989

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6b3245d
ABC
akhtarmdsaad Jan 28, 2024
77434ce
Fix issue #3972
akhtarmdsaad Feb 18, 2024
e0bfab1
Added markup test
akhtarmdsaad Feb 18, 2024
695e52c
Updated Changes.md file
akhtarmdsaad Feb 18, 2024
457ce3d
Removed unnecessary modes
akhtarmdsaad Feb 24, 2024
354c814
Merge branch 'main' into fix/issue-3972
akhtarmdsaad Feb 24, 2024
3f616e9
Fixed ts_specific keywords
akhtarmdsaad Feb 25, 2024
fcf18a7
Added the reason for the change
akhtarmdsaad Feb 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ New Grammars:

Core Grammars:

- fix(cpp) fixed highlighter break state [Md Saad Akhtar][]
- fix(rust) added negative-lookahead for callable keywords `if` `while` `for` [Omar Hussein][]
- enh(armasm) added `x0-x30` and `w0-w30` ARMv8 registers [Nicholas Thompson][]
- enh(haxe) added `final`, `is`, `macro` keywords and `$` identifiers [Robert Borghese][]
Expand Down
12 changes: 10 additions & 2 deletions src/languages/typescript.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,18 @@ export default function(hljs) {
"unknown"
];
const NAMESPACE = {
beginKeywords: 'namespace',
begin: [
/namespace/,
/\s+/,
hljs.IDENT_RE
],
beginScope: {
1: "keyword",
3: "variable"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the examples (https://www.typescriptlang.org/docs/handbook/namespaces.html) it looks like namespaces are class-like and CamelCase in their naming... so I think title.class would be more appropriate here than variable.

},
end: /\{/,
excludeEnd: true,
contains: [ tsLanguage.exports.CLASS_REFERENCE ]
contains: [tsLanguage.exports.CLASS_REFERENCE]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we need any of this anymore - the match rule should be enough to highlight the keyword and namespace name... but should IDENT_RE used above to recognize namespace perhaps be CLASS_REFERENCE[:match] instead - if the same names rules apply.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

const NAMESPACE = {
    begin: [
      /namespace/,
      /\s+/,
      hljs.IDENT_RE
    ],
    beginScope: {
      1: "keyword",
      3: "title.class"
    }
  };

Is the above change enough or i should change something else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was suggesting tsLanguage.exports.CLASS_REFERENCE instead of IDENT_RE... but yes that's looking good.

};
const INTERFACE = {
beginKeywords: 'interface',
Expand Down
4 changes: 4 additions & 0 deletions test/markup/typescript/namespace_as_identifier.expect.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

<span class="hljs-keyword">const</span> message = <span class="hljs-string">&#x27;foo&#x27;</span>;
<span class="hljs-keyword">const</span> <span class="hljs-keyword">namespace</span> = <span class="hljs-string">&#x27;bar&#x27;</span>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the desire behavior... We'll also need to remove it from TS_SPECIFIC_KEYWORDS (lets just comment it out - and mention it has a separate rule below).

<span class="hljs-keyword">function</span> <span class="hljs-title function_">baz</span>(<span class="hljs-params"></span>) {}
3 changes: 3 additions & 0 deletions test/markup/typescript/namespace_as_identifier.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const message = 'foo';
const namespace = 'bar';
function baz() {}
Loading