beginning-of-defun et al. not behaving the same as on C++ mode

[mcraveiro]

Hi csharp-mode developers,

thanks very much for an extremely useful mode, which I rely on on a daily basis. I found a small niggle, I think. When in C++ mode I can go to the start of a "defun" with C-M-a and end with C-M-e, and expand region also works correctly. However, in C# mode, C-M-e/C-M-a seem to take me to the namespace definition rather than the class and function definitions. Are these keys not mapped in C# mode perhaps?

Many thanks for your time

[josteink]

The keys are bound, but the behaviour of beginning-of-defun and end-of-defun is major-mode specific.

And it seems we're not implementing either of those correctly. Not in the re-implemented-from-scratch csharp-mode, nor in the even-newer csharp-tree-sitter-mode.

@theothornhill: My suggestion about upstreaming tree-sitter-mode functions only made sense as long as the maintainer had time and attention to give to that effort.

If we for now make csharp-tree-sitter-mode specific implementations for simple functions like beginning-of-defun and end-of-defun I think that's an OK way to get started exploiting this power.

When/if we have things upstreamed, we can rewrite to exploit that later and reduce the amount of code we have to maintain.

To that end, I've taken your code, touched it up slightly and added it to the feature/tree-sitter-navigation branch.

@mcraveiro : Would you be able to test if csharp-tree-sitter-mode works for you in that branch, and if it does what you expect?

[mcraveiro]

Thanks for your prompt reply. I can try csharp-tree-sitter-mode, but I am on windows - does it require installing any additional packages? I'm restricted on downloads unfortunately :-(

[josteink]

That new version depends on this package, which hopefully should contain everything we need:

https://melpa.org/#/tree-sitter

Edit: Also this: https://melpa.org/#/tree-sitter-langs

[theothornhill]

@josteink Very nice!

I'm having some tree-sitter issues on the M1 mac as for now, so trying to enable support for that - however that seems to take some time. I'll try to test the functionality when I get some free time, though.

I also think we should try to create some nice functionalities with this - I'll look into your branch also :)

[josteink]

Based on the feedback I got on my tree-sitter issue, it seems like we have some unbalanced parenthesis in our grammar somewhere which earlier versions of the tree-sitter runtime handled better.

I'll go hunting for it and hopefully land a fix.

[theothornhill]

From how I read that issue, it could be this part?

   [(string_literal)
    (verbatim_string_literal)
    (interpolated_string_text)
    (interpolated_verbatim_string_text)
    (character_literal)
    "\""
    "$\""
    "@$\""
    "$@\""] @string

They are needed for string fontification, and aren't nodes by themselves (at least when I added them). What do you think?

[josteink]

I'm in the process of bisecting it.

I'll push some changes to an intermediate branch when I'm back to something working.

[josteink]

This runs with latest versions of all dependent packages: https://github.com/emacs-csharp/csharp-mode/tree/bugfix/tree-sitter-crashes

[theothornhill]

Nice - thanks for isolating. Do we have any indication as to why they crash us? Also, out of curiosity, why is the setq helping here?

I vote for just removing these, I don't think they are super important - and we could easily add similar patterns down the line when we feel the need.

[josteink]

Also, out of curiosity, why is the setq helping here?

It's not, but it helps if you are going to re-evalutate the buffer about 200 times to test different versions of the grammar. And I don't see the need for this to be a var to be honest 😄

Nice - thanks for isolating.

This was a lot easier with a setq 😉

Do we have any indication as to why they crash us?

None what so ever. It's not even consistent with the handing of parsing-errors in upstream tree-sitter 0.19.3 (wrt unbalanced parenthesis), because I don't think that's possible for us to implement in elisp.

We may consider reporting our findings back upstream, but right now I don't have time for that, and just need a working c# setup in my Emacs.

[josteink]

I vote for just removing these, I don't think they are super important

I'll make it happen.

[theothornhill]

I vote for just removing these, I don't think they are super important

I'll make it happen.

Great! Yeah, I'm fine with the setq, though it should work with C-M-x :) Nice work!

[josteink]

All this should now be in latest MELPA.

@mcraveiro: Can you retest with the latest package version now?

[mcraveiro]

Perfect! amazing job guys!