Re: Remove js--treesit-backward-up-list or fix it

Re: Remove js--treesit-backward-up-list or fix it

[Theodor Thornhill via Emacs development discussions.]

(added back in emacs-devel)

Hi!

>> 
>> ```
>> const fooClient = new Foo(() => ({
>>                            | // <---- point is there  
>>                          })); 
>> ```
>> Which is not what we want.  If we can keep that correct behavior in any
>> case I say we remove the hack.  We could make an anchor helper named
>> something like 'parent-then-bol', but because there is no node here, we
>> cannot find the parent.
>> 
>> What do you think?

>
> I added a rule that fixes it. Actually, when there is no node at
> point, parent is set to the smaller node that spans point, kind of
> like the immediate “parent” of that point. I’ve added some explanation
> in the manual.
>

Yes, It's probably a remnant from when the semantics were a bit
different between the treesit-node-on and treesit-node-at.  I see it was
never there in ts-mode ;-)

>> 
>> However, there are some problems with the newest revisions.  During
>>      })
>>      .catch((r) => {
>>        if (r.status < 200 || r.status >= 300) {
>>          console.log(r);
>>        }
>>      });
>>  };
>> ```
>
> Yes, sorry. I was naive with my implementation. Now it should be fixed.
>

Absolutely!  Great work!

However... Try creating a js-mode buffer and type:

```
const foo = foo(() => {});
```

Because of all the stuff we set before we init tree sitter we get a
super unwanted behavior of the cursor jumping back to indentation for
every () {} etc. This is because of these settings, which are IMO pretty
bad defaults:
```
  (setq-local electric-indent-chars
	      (append "{}():;," electric-indent-chars)) ;FIXME: js2-mode adds "[]*".
  (setq-local electric-layout-rules
	      '((?\; . after) (?\{ . after) (?\} . before)))
```

This could just be something forgotten in the indent implementation, but
in my book this is yet another example for why we shouldn't init like
this ;-)

-- 
Theo

Re: Remove js--treesit-backward-up-list or fix it

[Yuan Fu]

> On Oct 29, 2022, at 11:31 AM, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> 
> (added back in emacs-devel)
> 
> Hi!
> 
>>> 
>>> ```
>>> const fooClient = new Foo(() => ({
>>>                           | // <---- point is there  
>>>                         })); 
>>> ```
>>> Which is not what we want.  If we can keep that correct behavior in any
>>> case I say we remove the hack.  We could make an anchor helper named
>>> something like 'parent-then-bol', but because there is no node here, we
>>> cannot find the parent.
>>> 
>>> What do you think?
> 
>> 
>> I added a rule that fixes it. Actually, when there is no node at
>> point, parent is set to the smaller node that spans point, kind of
>> like the immediate “parent” of that point. I’ve added some explanation
>> in the manual.
>> 
> 
> Yes, It's probably a remnant from when the semantics were a bit
> different between the treesit-node-on and treesit-node-at.  I see it was
> never there in ts-mode ;-)

That was actually deliberate semantic (which I forgot to document), I broke it when introducing the new semantic for treesit-node-at...

> 
>>> 
>>> However, there are some problems with the newest revisions.  During
>>>     })
>>>     .catch((r) => {
>>>       if (r.status < 200 || r.status >= 300) {
>>>         console.log(r);
>>>       }
>>>     });
>>> };
>>> ```
>> 
>> Yes, sorry. I was naive with my implementation. Now it should be fixed.
>> 
> 
> Absolutely!  Great work!
> 
> However... Try creating a js-mode buffer and type:
> 
> ```
> const foo = foo(() => {});
> ```
> 
> Because of all the stuff we set before we init tree sitter we get a
> super unwanted behavior of the cursor jumping back to indentation for
> every () {} etc. This is because of these settings, which are IMO pretty
> bad defaults:
> ```
>  (setq-local electric-indent-chars
> 	      (append "{}():;," electric-indent-chars)) ;FIXME: js2-mode adds "[]*".
>  (setq-local electric-layout-rules
> 	      '((?\; . after) (?\{ . after) (?\} . before)))
> ```
> 
> This could just be something forgotten in the indent implementation, but
> in my book this is yet another example for why we shouldn't init like
> this ;-)

I don’t have anything intelligent to say about the init process (except that we definitely need a way for js2-mode to turn off tree-sitter setup made by js-mode even if it is enabled by the user), but I fixed the indent problem you observed :-)

Yuan

Re: Remove js--treesit-backward-up-list or fix it

[Theodor Thornhill via Emacs development discussions.]

>
> I don’t have anything intelligent to say about the init process
> (except that we definitely need a way for js2-mode to turn off
> tree-sitter setup made by js-mode even if it is enabled by the user),
> but I fixed the indent problem you observed :-)
>

Ah you were faster than me.  Well, you can disregard most of the patch I
just sent to bug-gnu-emacs, but we still need the fix in parent-bol.  We
need back-to-indentation+point, not current-indentation, don't we?

-- 
Theo

Re: Remove js--treesit-backward-up-list or fix it

[Yuan Fu]

> On Oct 29, 2022, at 12:31 PM, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> 
>> 
>> I don’t have anything intelligent to say about the init process
>> (except that we definitely need a way for js2-mode to turn off
>> tree-sitter setup made by js-mode even if it is enabled by the user),
>> but I fixed the indent problem you observed :-)
>> 
> 
> Ah you were faster than me.

A whip named shame was chasing after me.

>  Well, you can disregard most of the patch I
> just sent to bug-gnu-emacs, but we still need the fix in parent-bol.  We
> need back-to-indentation+point, not current-indentation, don't we?

Right. My brain is not having its best time today :-) I applied your change. Thanks!

Yuan

Re: Remove js--treesit-backward-up-list or fix it

[Theodor Thornhill via Emacs development discussions.]

On 29 October 2022 22:08:39 CEST, Yuan Fu <casouri@gmail.com> wrote:
>
>
>> On Oct 29, 2022, at 12:31 PM, Theodor Thornhill <theo@thornhill.no> wrote:
>> 
>> 
>>> 
>>> I don’t have anything intelligent to say about the init process
>>> (except that we definitely need a way for js2-mode to turn off
>>> tree-sitter setup made by js-mode even if it is enabled by the user),
>>> but I fixed the indent problem you observed :-)
>>> 
>> 
>> Ah you were faster than me.
>
>A whip named shame was chasing after me.
>

Hah! 

>>  Well, you can disregard most of the patch I
>> just sent to bug-gnu-emacs, but we still need the fix in parent-bol.  We
>> need back-to-indentation+point, not current-indentation, don't we?
>
>Right. My brain is not having its best time today :-) I applied your change. Thanks!
>
>

:-)

No worries!

Theo

Re: Remove js--treesit-backward-up-list or fix it

[Stefan Monnier]

> Because of all the stuff we set before we init tree sitter we get a
> super unwanted behavior of the cursor jumping back to indentation for
> every () {} etc. This is because of these settings, which are IMO pretty
> bad defaults:
> ```
>   (setq-local electric-indent-chars
> 	      (append "{}():;," electric-indent-chars)) ;FIXME: js2-mode adds "[]*".
>   (setq-local electric-layout-rules
> 	      '((?\; . after) (?\{ . after) (?\} . before)))
> ```

While these may be bad defaults, they should provide reasonable
behavior, so if you get cursor jumping with such settings, then we
should fix that regardless if its the default.


        Stefan

Re: Remove js--treesit-backward-up-list or fix it

[Theodor Thornhill via Emacs development discussions.]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Because of all the stuff we set before we init tree sitter we get a
>> super unwanted behavior of the cursor jumping back to indentation for
>> every () {} etc. This is because of these settings, which are IMO pretty
>> bad defaults:
>> ```
>>   (setq-local electric-indent-chars
>> 	      (append "{}():;," electric-indent-chars)) ;FIXME: js2-mode adds "[]*".
>>   (setq-local electric-layout-rules
>> 	      '((?\; . after) (?\{ . after) (?\} . before)))
>> ```
>
> While these may be bad defaults, they should provide reasonable
> behavior, so if you get cursor jumping with such settings, then we
> should fix that regardless if its the default.
>

Yuan fixed it :-)

-- 
Theo