indents(ecma): use @auto for comments

[theHamsta]

Fixes #2544

@ajitid

[ajitid]

Hey @theHamsta, do you have a GIF/screenshot showing that it is working? Because it is still broken for me with this PR. Here's what I did:

1. Replaced

Plug 'nvim-treesitter/nvim-treesitter', {'do': ':TSUpdate'}

with

Plug 'theHamsta/nvim-treesitter', {'branch': 'ecma-auto-comment', 'do': ':TSUpdate'}

2. Just for surety, I did :TSUninstall all and restarted nvim so that it could freshly install parsers.

Edit: you made a push right after I dropped this comment. Let me check the changes done in force push ( 54e4530) and let me get back to you...

[ajitid]

It is not working for me with 54e4530

JS file:

TS file:

TSX file:

As you can see, all three have different behavior, TSX being closest to stock Vim one (Vim also automatically joins comment's end string — * / becomes */).

[theHamsta]

It's working for me and it's also working in the tests. Did you restart?

It will not work when starting a comment with /* as it's taking it for a (ERROR (binary_expression (regex)))

fun fact: tsx is the only of the filetypes that wouldn't automatically set * in the tests

[ajitid]

It will not work when starting a comment with /* as it's taking it for a (ERROR (binary_expression (regex)))

Just to clarify — you're meaning to say that text shouldn't be written on the line where starting comment string is /*, right? If yes, then I'm not writing text there.

Did you restart?

I did restart. I'll try to remove everything and check again

[theHamsta]

Me and the tests have the behavior of your TSX in JS and TS. The indents are almost the same for all three languages. There has to be some difference in the Vimscript filetype configuration.

[ajitid]

@theHamsta I just tried with basically removing everything (here's the dotfile I used for that matter) and I can still see the same issue.

[theHamsta]

But you're not using our indent module in your config

nvim-treesitter/scripts/minimal_init.lua

Line 12 in 2298a75

indent = { enable = true },

but because we disable syntax it's possible that we break one of the traditional indent methods

[ajitid]

Oh, my mistake! I tried, now it works for all files except there's a slight issue in .ts ones. In them, I had to manually align ending multiline comment (look for */ in the GIF):

[theHamsta]

Yes, this is the issue I was speaking of before. At the moment when we have just /* it's a regex in a error node for tree-sitter. I tried to match it but until now without success. Maybe we could use @auto always on errors.

[ajitid]

Oh okay. Weird that it only occurs in .ts but not in .js or .tsx files, isn't it? Anyway, I'm quite happy as we've made a significant progress here.

Maybe we could use @auto always on errors.

Totally on you if you want to merge the PR and close the issue or to give another stab fixing it. Thanks for your efforts!

[theHamsta]

@ajitid can you check again? The behavior for unfinished /* should be fixed now.

The behavior for all three filetypes is the same (see tests)

[theHamsta]

No idea why tests are failing without error. Seems similar to #2291

[ajitid]

Here's what I got (all of these GIFs are of a .tsx file):

VS Code's multiline comment:

vs this PRs multiline comment:

vs what stock Vim gives us:

Stock Vim behaves in an odd fashion, so let's keep it aside for now. But this PR has different indent behavior when a comment is made at the first column vs when it is done at an indent.

Here's VSCode's JSDoc comment:

To me VSCode's behavior seem to be the most appropriate here.

---

Apologies I'm not the best person to verify validity of the tests written.

[theHamsta]

Hmm, apparently it depends on the context on how the ERROR because it will add anything before to the error so that our regex will not match. I will try multi-line Lua regexes. I now works in more cases.

I find your GIFs super unhelpful. It's difficult to understand at what moment something should behave differently. It's usually better to have plain text with a marker where something should be inserted and what the result after indentation will be. Plus I can copy+paste your complete failure case to make it a test.

[ajitid]

I find your GIFs super unhelpful. It's difficult to understand at what moment something should behave differently. It's usually better to have plain text with a marker where something should be inserted and what the result after indentation will be. Plus I can copy+paste your complete failure case to make it a test.

Noted. Will use text from now on.

[theHamsta]

@ajitid I didn't mean in any way to criticize you. I'm very glad that you discovered that bug also in the "fixes". We can just save us some round-trips when it's clear what we mean. Best to communicate is failing example or even better a test for that.

[theHamsta]

todo: fix regex

[icholy]

I just tested this branch and it still seems broken to me.

If the comment isn't closed, the next line does not get indented correctly.

/** <- hit [ENTER] here
*

If the comment is closed, the next line gets indented correctly.

/** <- hit [ENTER] here
 *
 */

[khamer]

I ran into this, switched to @theHamsta 's fork and it resolved the issues for me. I know it's nearly a thousand commits behind master now; what's the status here? Anything I can do to help? Any chance of a merge? At worst, I might see about rebasing the fork on top of master for my own use.

[theHamsta]

I don't remember whether this was an improvement or regression. Since the tests pass I guess it's ok? Can other JS please comment? I can maybe give it a try to solve the remaining issues that we still had here.

[khamer]

I'm not sure - the comment indentation has been wrong since I started using this, so it might be an improvement. Compared to how C-style block comment indentation is handled w/o treesitter, it's a regression.

[theHamsta]

I think we should merge. It is the same how comments are currently handled in C/C++ and Java. Though Go still uses the @ignore way.

[ObserverOfTime]

LGTM aside from that one thing.

[ObserverOfTime]

Isn't this unnecessary when (ERROR) @auto already exists above?

[amaanq]

ping @theHamsta this would be really nice to merge :)

[clason]

Needs to be adapted to the new upstream format, if still desired.

[theHamsta]

comment, (ERROR) are already @indent.auto in master, template_string already @indent.ignore. The rule that matches inside the error is probably not stable and unclear whether relevant for current parser.