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.

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

[JavaScript] fix tagged template string key bindings regression #4022

Merged

Conversation

deathaxe
Copy link
Collaborator

@deathaxe deathaxe commented Jul 28, 2024

This PR...

  1. scopes tagged template strings meta.string.template to give all of them a common scope without regards of used syntax highlighting
  2. uses that scope in key bindings' selectors to enable them in syntax highlighted tagged templates.
  3. restrict ` to JavaScript strings by enforcing scope order in selectors in binding 1 and 2
  4. merge "selector" keys in binding 3 and 4

It fixes no longer working key bindings in syntax highlighted tagged templates, regressed by #4019.

This commit...

1. scopes tagged template strings `meta.string.template` to give all of them
   a common scope without regards of used syntax highlighting
2. uses that scope in key bindings' selectors to enable them in syntax
   highlighted tagged templates.
3. restrict ` to JavaScript strings by enforcing scope order in selectors
   in binding 1 and 2
4. merge "selector" keys in binding 3 and 4
@deathaxe
Copy link
Collaborator Author

It's currently a bit unclear to me, whether auto-pairing backquotes not being enabled in JSX/TSX is by intent or whether we want to use the opportunity to add it.

This commit adds a `enter` key binding to achieve the following, when hitting
enter within two backquotes.

before:

    foo = `|`

after:

    foo = `
        |
    `
This commit moves `selector` after `settings` and `selection_empty`
as it seems to be the commonly used order in ST's default key bindings.
…ecks

This commit aligns selector checks with ST's default key bindings by
adding `"match_all": true` to all selector context checks.
This commit removes default entries.

-  operator: equal
-  operand: true
deathaxe added a commit to deathaxe/sublime-packages that referenced this pull request Jul 29, 2024
This commit changes `enter` key binding to "Add Line in Braces" macro,
as it indents line between backticks using indentation rules instead of
forcing it to be indented via `\t`.

This works also, if indentation rules decide not to indent backticked contents.
@deathaxe deathaxe force-pushed the pr/javascript/fix-template-keybindings branch from 348787b to 1a1c9cc Compare August 17, 2024 15:36
@deathaxe deathaxe changed the title [JavaScript] Adjust tagged template string related key bindings [JavaScript] fix tagged template string key bindings regression Aug 17, 2024
@FichteFoll
Copy link
Collaborator

It's currently a bit unclear to me, whether auto-pairing backquotes not being enabled in JSX/TSX is by intent or whether we want to use the opportunity to add it.

I suspect this was an oversight as I see no reason to treat them differently, personally.

@deathaxe deathaxe merged commit 89155fe into sublimehq:master Aug 26, 2024
1 check passed
@deathaxe deathaxe deleted the pr/javascript/fix-template-keybindings branch August 26, 2024 06:45
FichteFoll added a commit that referenced this pull request Aug 30, 2024
* [JavaScript] Exclude mappings from normal keyword indentation

Fixes https://forum.sublimetext.com/t/sublime-text-on-the-word-default/72953

This commit applies dedicated (json) indentation rules to JavaScript mappings
to avoid unexpected (un-)indenting of keys, which look like reserved control
structure keywords (e.g.: case, default, if, else, ...).

* [JavaScript] Exclude lists from normal keyword indentation

Same as for mappings, is true for lists. They can contain otherwise reserved
words, which should not be treated by indentation rules.

* [JavaScript] Add key binding for bracket content auto-indentation

This commit applies a key binding from JSON package to auto-indent content
of lists when hitting enter

    var = [|]

becomes:

    var = [
        |
    ]

* [JavaScript] Handle function calls in mappings or lists

This commit...

1. extends selectors for each syntax (JS, JSX, TS, TSX) to choose correct
   indentation rules in nested mappings, lists or functions/lambdas.

2. Adds check for trailing `:` to case/default indentation rule patterns,
   as a function called `case()` in a mapping can't be prevented from being
   unindented via selectors.

* [JavaScript] Reorganize tests

Merge new tests into existing file

* [JavaScript] Add tests for embedded JS code blocks

* [JavaScript] Remove failing tests

As we need to expect `:` after case/default, those tests no longer work
by intent.

* [JavaScript] Rename indentation rules file

* [JavaScript] Improve indentation rules of tagged template strings

This commit...

1. fixes lines after single-line tagged template string being indented.

   before:

   var foo = html`<p>content</p>`;
       var bar = html`<p>content</p>`;
           var baz = html`<p>content</p>`:

   after:

   var foo = html`<p>content</p>`;
   var bar = html`<p>content</p>`;
   var baz = html`<p>content</p>`:

2. excludes content of plain string templates from auto-indentation.
   As their kind of content is undefined, normal indentation rules may
   cause false results.

   Tests are added to ensure normal JS statements keep untreated.

Notes:
   First and last line of tagged templates is scoped `string.quoted.other`
   regardless of embedded syntax highlighting, if only opening or closing
   backticks appear.

   Hence increasing indentation of tagged template strings' content is handled
   by "Indentation Rules - Template Strings".

   Decreasing indentation of closing backtick is handled
   by normal "Indentation Rules" as those are the ones applied by ST.
   It just doesn't work otherwise.

   This circumstance helps to reliably distinguish opening and closing
   backticks and correctly in- or decrease indentation.

* [JavaScript] Tweak key binding context key order

This commit aligns this PR with #4022.

* [JavaScript] Tweak enter key binding

This commit changes `enter` key binding to "Add Line in Braces" macro,
as it indents line between backticks using indentation rules instead of
forcing it to be indented via `\t`.

This works also, if indentation rules decide not to indent backticked contents.

* [JavaScript] Simplify indentation rules

By utilizing the improved selector scoring of build 4173, we can simply
have the two rules override each other based on the last meta scope on
the stack.

---------

Co-authored-by: FichteFoll <[email protected]>
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.

3 participants