Skip to content

[JavaScript/TypeScript] Improve shebang and comments highlighting #3018

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.

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

Conversation

deathaxe
Copy link
Collaborator

@deathaxe deathaxe commented Sep 5, 2021

Fixes #2880

This PR implements ...

  1. shebang highlighting using the scheme of PHP/Java/ShellScript to ensure it being highlighted at the very first line, only.
  2. xml highlighting in triple-slash comments in TS/TSX syntax according to Typescript specs, but with a more lazy/forgiving xml highlighting.
  3. more forgiving comment punctuation highlighting in order to help ligatures for // or /// to be displayed and to improve UX in case color schemes apply special highlighting to comment punctuation.
  4. scopes trailing // in line comments punctuation for consistent highlighting.
  5. adds various comment related test cases to illustrate changes and differences between syntaxes.

It feels odd to only highlight

  • /* and */ as punctuation in delimiters like /***************************/
  • // in /// comment or /////////////////////////////////////

Note:

If double-slash and triple-slash sub-scopes are not needed, all line comment related contexts could probably be simplified/merged in some way.

This commit refactors the way how shebang context is implemented after
the scheme of PHP/Java/ShellScript.

It makes sure to only highlight a comment as shebang in the very first
line. The only downside is it not being testable anymore.
Fixes sublimehq#2880

This commit adds xml-tag syntax highlighting in triple-slash line
comments at the beginning of a document.

XML syntax is embedded in order to ...
1. enforce escaping on EOL
2. prevent any prototype from being injected into XML
This commit prepends a pattern to `comments` to consume tiple-slash
line comments in the middle of a typescript document in order to enable
ST to properly display triple-slash ligatures in the same way as they
are now displayed in directive comments at the beginning of a document.
This commit...

1. moves triple-slash comment contexts to JavaScript in order to
   highlight comments equally in all JS/TS/JSX/TSX derivatives.
2. splits block and line comment contexts
3. applies some changes to highlight arbitrary amount of slashes
   or asterisks as comment punctuation. This is already applied to
   some other syntax definitions and intents to
   a) provide a more consistent highlighting in case a color scheme
      defines dedicated rules for comment punctuation.
   b) improve ligatures detection/grouping
4. A pattern for empty block comments is added to avoid complicated and
   probably error prone negative lookaheads in case of `/**/` or `/***/`
   scenarios in which the opening and closing punctuation can't be
   determined properly.

Notes:
1. The overall strategy with regards to block comments is borrowed from
   PR sublimehq#2654.
2. Line comments could be simplified if `double-slash`, `triple-slash`
   sub scopes are considered unnecessary.
@deathaxe deathaxe requested review from Thom1729 and wbond September 5, 2021 10:12
This commit adds some prototype exclusions to ensure shebang comments
to be highlighted at the very first line only.
wbond
wbond previously approved these changes Sep 5, 2021
Copy link
Member

@wbond wbond left a comment

Choose a reason for hiding this comment

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

Looks good to me. Only thing I wasn't sure about was the Erlang comment, which I am guessing was a copy-paste error?

@deathaxe deathaxe force-pushed the pr/javascript/fix-shebang-preprocessor-comments branch from 2f763f0 to 6fc2f9b Compare September 5, 2021 13:31
This commit is borrowed from Java to scope trailing slashes in line
comments as punctuation in case someone wants to use them as separator
with title.
@deathaxe deathaxe force-pushed the pr/javascript/fix-shebang-preprocessor-comments branch from 6fc2f9b to d6b7d06 Compare September 5, 2021 13:33
@deathaxe
Copy link
Collaborator Author

deathaxe commented Sep 5, 2021

Fixed the typo. While doing so I found Java (PR) to also highlight trailing slashes in line comments. Added it as it might look better to have all slashes scoped the same in // title ///////.

@deathaxe deathaxe requested a review from wbond September 5, 2021 13:36
Thom1729
Thom1729 previously approved these changes Sep 5, 2021
@keith-hall
Copy link
Collaborator

Fixed the typo. While doing so I found Java (PR) to also highlight trailing slashes in line comments. Added it as it might look better to have all slashes scoped the same in // title ///////.

Just a quick check - I recall that wrap_line functionality relies on the comment punctuation characters being scoped. Have you tested to ensure that nothing breaks when more characters in the comment receive these scopes (both leading and trailing punctuation)?

@deathaxe
Copy link
Collaborator Author

deathaxe commented Sep 6, 2021

Tried wrap_line in C and C# in SAFE MODE and it doesn't add another // when hitting enter at the end of a line comment. The only thing I've seen is * being added in block comments.

I have no strong opinion in applying such stuff to line comments. I just added it here, because there was another syntax (forgot which one), I had a discussion about different types of line comments, which ended up in exactly such rules.

Scoping trailing slashes or all is just to highlight "delimiting comments" which span the whole line, consistently. Maybe it's not worth it. Thoughts?

We should at least distinguish double-slash and triple-slash in a sane and consistent way as both have different meaning in TypeScript and most fonts provide ligatures for them.


EDIT: Just realized wrap_line to be the one which is called by alt+q.

The comment:

///// Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

After wrap_line in current JavaScript

///// Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut
///// labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco
///// laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in
///// voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat
///// cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

After wrap_line in current C#

///// Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut
///   labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco
///   laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in
///   voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat
///   cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

Which one looks better??

@keith-hall
Copy link
Collaborator

Scoping trailing slashes or all is just to highlight "delimiting comments" which span the whole line, consistently. Maybe it's not worth it. Thoughts?

I personally like the idea, there may be color schemes which highlight a comment's punctuation differently from the comment text

Which one looks better??

Consistency wins IMHO i.e. your current JavaScript :)

@deathaxe deathaxe requested a review from Thom1729 September 12, 2021 12:40
@deathaxe deathaxe merged commit b4744b9 into sublimehq:master Sep 12, 2021
@deathaxe deathaxe deleted the pr/javascript/fix-shebang-preprocessor-comments branch September 12, 2021 15:29
@deathaxe
Copy link
Collaborator Author

Thank's for having a look on it.

deathaxe added a commit to deathaxe/sublime-packages that referenced this pull request Jan 27, 2022
Fixes sublimehq#3215

This commit improves comment punctuation highlighting to support
arbitrary number of opening and closing punctuation chars.

This helps to keep line comment punctuation consistent when performing
hard line wrapping.

The new contexts are copied from JavaScript (see sublimehq#3018) and adjusted to
match C's scopes and maintain the existing "banner comments".
mitranim pushed a commit to mitranim/Packages that referenced this pull request Mar 25, 2022
…blimehq#3018)

* [JavaScript] Refactor shebang highlighting

This commit refactors the way how shebang context is implemented after
the scheme of PHP/Java/ShellScript.

It makes sure to only highlight a comment as shebang in the very first
line. The only downside is it not being testable anymore.

* [TypeScript] Add xml highlighting in triple-slash comments

Fixes sublimehq#2880

This commit adds xml-tag syntax highlighting in triple-slash line
comments at the beginning of a document.

XML syntax is embedded in order to ...
1. enforce escaping on EOL
2. prevent any prototype from being injected into XML

* [TypeScript] Fix ligatures in triple-slash comments

This commit prepends a pattern to `comments` to consume tiple-slash
line comments in the middle of a typescript document in order to enable
ST to properly display triple-slash ligatures in the same way as they
are now displayed in directive comments at the beginning of a document.

* [JavaScript] Refactor comments

This commit...

1. moves triple-slash comment contexts to JavaScript in order to
   highlight comments equally in all JS/TS/JSX/TSX derivatives.
2. splits block and line comment contexts
3. applies some changes to highlight arbitrary amount of slashes
   or asterisks as comment punctuation. This is already applied to
   some other syntax definitions and intents to
   a) provide a more consistent highlighting in case a color scheme
      defines dedicated rules for comment punctuation.
   b) improve ligatures detection/grouping
4. A pattern for empty block comments is added to avoid complicated and
   probably error prone negative lookaheads in case of `/**/` or `/***/`
   scenarios in which the opening and closing punctuation can't be
   determined properly.

Notes:
1. The overall strategy with regards to block comments is borrowed from
   PR sublimehq#2654.
2. Line comments could be simplified if `double-slash`, `triple-slash`
   sub scopes are considered unnecessary.

* [JavaScript] Restrict shebang highlighting to very first line

This commit adds some prototype exclusions to ensure shebang comments
to be highlighted at the very first line only.

* [JavaScript] Scope trailing slashes in line comments

This commit is borrowed from Java to scope trailing slashes in line
comments as punctuation in case someone wants to use them as separator
with title.

* [JavaScript] Rename javascript context to script
deathaxe added a commit that referenced this pull request Sep 24, 2023
Fixes #3215

This PR improves comment punctuation highlighting to support 
arbitrary number of opening and closing punctuation chars.

This helps to keep line comment punctuation consistent when performing hard line wrapping.

The new contexts are copied from JavaScript (see #3018) and adjusted to match C's scopes
and maintain the existing "banner comments".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TypeScript] Can't match triple slash directives in the Typescript
5 participants