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] Code and Data specific indentation rules #4015

Merged
merged 14 commits into from
Aug 30, 2024

Conversation

deathaxe
Copy link
Collaborator

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, ...).

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, ...).
michaelblyons
michaelblyons previously approved these changes Jul 26, 2024
@rchl
Copy link
Contributor

rchl commented Jul 26, 2024

Was curious if it changes the behavior I struggle with often. It looks like it does. It's not ideal either before or after but it does change the behavior. Is it intended?

(to clarify, I'm pressing enter within an empty array)

Before

Screenshot 2024-07-26 at 14 10 50

After

Screenshot 2024-07-26 at 14 09 50

Personally I feel like the new behavior is easier to correct manually since I just need to press enter again, then up and tab.

@deathaxe
Copy link
Collaborator Author

This change is intended to get the following:

grafik

I now realize it is me seeing that due to a custom key binding of mine, to fix bracket behavior.

	// Move closing bracket down by 2 lines and place caret onto next line with indentation
	{ "keys": ["enter"], "command": "insert_snippet", "args": {"contents": "\n\t$0\n"}, "context":
		[
			{ "key": "setting.auto_indent", "operator": "equal", "operand": true },
			{ "key": "selection_empty", "operator": "equal", "operand": true, "match_all": true },
			{ "key": "preceding_text", "operator": "regex_contains", "operand": "\\[$", "match_all": true },
			{ "key": "following_text", "operator": "regex_contains", "operand": "^\\]", "match_all": true }
		]
	},

Same as for mappings, is true for lists. They can contain otherwise reserved
words, which should not be treated by indentation rules.
This commit applies a key binding from JSON package to auto-indent content
of lists when hitting enter

    var = [|]

becomes:

    var = [
        |
    ]
@deathaxe
Copy link
Collaborator Author

Added a commit to also exclude keyword indentation treatment from meta.sequence and applied the key binding from JSON.

Can you check, whether it improves your situation, @rchl ?

keith-hall
keith-hall previously approved these changes Jul 26, 2024
JavaScript/Default.sublime-keymap Show resolved Hide resolved
JavaScript/Indentation Rules - Mappings.tmPreferences Outdated Show resolved Hide resolved
JavaScript/Indentation Rules.tmPreferences Outdated Show resolved Hide resolved
@rchl
Copy link
Contributor

rchl commented Jul 26, 2024

Added a commit to also exclude keyword indentation treatment from meta.sequence and applied the key binding from JSON.

Can you check, whether it improves your situation, @rchl ?

It does work nicely now.

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.
Merge new tests into existing file
As we need to expect `:` after case/default, those tests no longer work
by intent.
@deathaxe deathaxe reopened this Jul 26, 2024
@deathaxe
Copy link
Collaborator Author

deathaxe commented Jul 26, 2024

Well, this PR now requires ST4175+ ST4173+.

It requires sublimehq/sublime_text#2152 in particular.

deathaxe added a commit to deathaxe/sublime-packages that referenced this pull request Jul 26, 2024
This change is required for sublimehq#4015 to succeed.

We can re-enable stable tests, once the next stable release is made.
deathaxe added a commit that referenced this pull request Jul 28, 2024
This change is required for #4015 to succeed.

Its indentation rule selectors make use of sublimehq/sublime_text#2152 
to recursively select the correct ruleset in deeply nested alternating json objects/lists and function constructs.

```js
var foo = {             
  case: () => {          // data: don't dedent
    switch () {
    case:                // code: dedent
      var bar = {
        case: () => {    // data: don't dedent
          ...
        }
      }        
   }
}
```

This is available as of ST4173.

In all builds before, selectors match the outermost contexts only
and thus indentation starts to fail in on of the inner object/function blocks.
keith-hall
keith-hall previously approved these changes Jul 28, 2024
michaelblyons
michaelblyons previously approved these changes Jul 28, 2024
@deathaxe deathaxe changed the title [JavaScript] Exclude mappings from normal keyword indentation [JavaScript] Code and Data specific indentation rules Jul 28, 2024
@deathaxe
Copy link
Collaborator Author

deathaxe commented Jul 28, 2024

Resolved a merge conflict in indentation rules.

keith-hall
keith-hall previously approved these changes Jul 28, 2024
michaelblyons
michaelblyons previously approved these changes Jul 29, 2024
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.
@deathaxe deathaxe dismissed stale reviews from michaelblyons and keith-hall via 67d86e6 July 29, 2024 10:19
@deathaxe
Copy link
Collaborator Author

Not sure if this PR becomes to big, but it seems useful to also fix a tagged template string related indentation issue, I've come accross while testing things out.

I found single line template strings to cause following lines to be increased in indentation by accident.

@FichteFoll
Copy link
Collaborator

Looks better now. I'd like to try this out and see if my idea to support arbitrary nesting of mapping literals and function definitions can also be achieved, but I can't promise when I'll find the time. Hopefully by the end of the week, though.

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-indentation branch from db36eca to a644c0e Compare August 17, 2024 15:38
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.
@FichteFoll
Copy link
Collaborator

Finally found the time to test this and it was a success. You can find the commit (and cherry-pick it) here: FichteForks@dfad440

Alternatively I can also push it to your branch, I believe, but I wanted you to review it first.

@deathaxe
Copy link
Collaborator Author

LGTM!

PRs are a place for collaboration. Feel free to directly push.

@FichteFoll FichteFoll merged commit bab8ccd into sublimehq:master Aug 30, 2024
1 check passed
@deathaxe deathaxe deleted the pr/javascript/fix-indentation branch August 30, 2024 16:51
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.

5 participants