unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
@ 2024-04-11 20:32 Jacob Leeming
  2024-04-12  5:45 ` Eli Zaretskii
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jacob Leeming @ 2024-04-11 20:32 UTC (permalink / raw)
  To: 70345

Hi all,

From emacs -Q:

Evaluate this elisp to set up treesitter for csharp:

(setq treesit-language-source-alist '((c-sharp
"https://github.com/tree-sitter/tree-sitter-c-sharp" "master" "src"))
      treesit-load-name-override-list '((c-sharp
"libtree-sitter-csharp" "tree_sitter_c_sharp"))
      major-mode-remap-alist '((csharp-mode . csharp-ts-mode)))

Insert the following text into a csharp-ts-mode buffer:

if (true)
var x = 2;

Try to indent the variable declaration of the function with
indent-for-tab-command. Nothing will happen. I'd expect to see this:

if (true)
    var x = 2;

This issue can be fixed with the following patch:

diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
index 53c52e6..1a7d535 100644
--- a/lisp/progmodes/csharp-mode.el
+++ b/lisp/progmodes/csharp-mode.el
@@ -678,7 +678,8 @@ csharp-ts-mode--indent-rules
      ((parent-is "binary_expression") parent 0)
      ((parent-is "block") parent-bol csharp-ts-mode-indent-offset)
      ((parent-is "local_function_statement") parent-bol 0)
-     ((parent-is "if_statement") parent-bol 0)
+     ((match "block" "if_statement") parent-bol 0)
+     ((parent-is "if_statement") parent-bol csharp-ts-mode-indent-offset)
      ((parent-is "for_statement") parent-bol 0)
      ((parent-is "for_each_statement") parent-bol 0)
      ((parent-is "while_statement") parent-bol 0)

Cheers,
Jacob





^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-11 20:32 bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body Jacob Leeming
@ 2024-04-12  5:45 ` Eli Zaretskii
  2024-04-14 23:02 ` Yuan Fu
  2024-04-15 19:01 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2024-04-12  5:45 UTC (permalink / raw)
  To: Theodor Thornhill, Yuan Fu; +Cc: 70345, Jacob Leeming

> From: Jacob Leeming <jacobtophatleeming@gmail.com>
> Date: Thu, 11 Apr 2024 21:32:02 +0100
> 
> Hi all,
> 
> >From emacs -Q:
> 
> Evaluate this elisp to set up treesitter for csharp:
> 
> (setq treesit-language-source-alist '((c-sharp
> "https://github.com/tree-sitter/tree-sitter-c-sharp" "master" "src"))
>       treesit-load-name-override-list '((c-sharp
> "libtree-sitter-csharp" "tree_sitter_c_sharp"))
>       major-mode-remap-alist '((csharp-mode . csharp-ts-mode)))
> 
> Insert the following text into a csharp-ts-mode buffer:
> 
> if (true)
> var x = 2;
> 
> Try to indent the variable declaration of the function with
> indent-for-tab-command. Nothing will happen. I'd expect to see this:
> 
> if (true)
>     var x = 2;
> 
> This issue can be fixed with the following patch:
> 
> diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
> index 53c52e6..1a7d535 100644
> --- a/lisp/progmodes/csharp-mode.el
> +++ b/lisp/progmodes/csharp-mode.el
> @@ -678,7 +678,8 @@ csharp-ts-mode--indent-rules
>       ((parent-is "binary_expression") parent 0)
>       ((parent-is "block") parent-bol csharp-ts-mode-indent-offset)
>       ((parent-is "local_function_statement") parent-bol 0)
> -     ((parent-is "if_statement") parent-bol 0)
> +     ((match "block" "if_statement") parent-bol 0)
> +     ((parent-is "if_statement") parent-bol csharp-ts-mode-indent-offset)
>       ((parent-is "for_statement") parent-bol 0)
>       ((parent-is "for_each_statement") parent-bol 0)
>       ((parent-is "while_statement") parent-bol 0)

Theo and Yuan, any comments?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-11 20:32 bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body Jacob Leeming
  2024-04-12  5:45 ` Eli Zaretskii
@ 2024-04-14 23:02 ` Yuan Fu
  2024-04-15  4:56   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-04-15 19:01 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2 siblings, 1 reply; 15+ messages in thread
From: Yuan Fu @ 2024-04-14 23:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70345, theo, jacobtophatleeming


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jacob Leeming <jacobtophatleeming@gmail.com>
>> Date: Thu, 11 Apr 2024 21:32:02 +0100
>> 
>> Hi all,
>> 
>> >From emacs -Q:
>> 
>> Evaluate this elisp to set up treesitter for csharp:
>> 
>> (setq treesit-language-source-alist '((c-sharp
>> "https://github.com/tree-sitter/tree-sitter-c-sharp" "master" "src"))
>>       treesit-load-name-override-list '((c-sharp
>> "libtree-sitter-csharp" "tree_sitter_c_sharp"))
>>       major-mode-remap-alist '((csharp-mode . csharp-ts-mode)))
>> 
>> Insert the following text into a csharp-ts-mode buffer:
>> 
>> if (true)
>> var x = 2;
>> 
>> Try to indent the variable declaration of the function with
>> indent-for-tab-command. Nothing will happen. I'd expect to see this:
>> 
>> if (true)
>>     var x = 2;
>> 
>> This issue can be fixed with the following patch:
>> 
>> diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
>> index 53c52e6..1a7d535 100644
>> --- a/lisp/progmodes/csharp-mode.el
>> +++ b/lisp/progmodes/csharp-mode.el
>> @@ -678,7 +678,8 @@ csharp-ts-mode--indent-rules
>>       ((parent-is "binary_expression") parent 0)
>>       ((parent-is "block") parent-bol csharp-ts-mode-indent-offset)
>>       ((parent-is "local_function_statement") parent-bol 0)
>> -     ((parent-is "if_statement") parent-bol 0)
>> +     ((match "block" "if_statement") parent-bol 0)
>> +     ((parent-is "if_statement") parent-bol csharp-ts-mode-indent-offset)
>>       ((parent-is "for_statement") parent-bol 0)
>>       ((parent-is "for_each_statement") parent-bol 0)
>>       ((parent-is "while_statement") parent-bol 0)
>
> Theo and Yuan, any comments?

If Theo is too busy I can take a look, but I don’t really know
csharp-ts-mode too well.

Yuan





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-14 23:02 ` Yuan Fu
@ 2024-04-15  4:56   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 15+ messages in thread
From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-15  4:56 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Eli Zaretskii, 70345, jacobtophatleeming

[-- Attachment #1: Type: text/html, Size: 3244 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-11 20:32 bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body Jacob Leeming
  2024-04-12  5:45 ` Eli Zaretskii
  2024-04-14 23:02 ` Yuan Fu
@ 2024-04-15 19:01 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-04-15 21:38   ` Jacob Leeming
  2024-04-22  8:50   ` Jacob Leeming
  2 siblings, 2 replies; 15+ messages in thread
From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-15 19:01 UTC (permalink / raw)
  To: Jacob Leeming; +Cc: 70345

> Hi all,
>

Hi, Jacob!

>>From emacs -Q:
>
> Evaluate this elisp to set up treesitter for csharp:
>
> (setq treesit-language-source-alist '((c-sharp
> "https://github.com/tree-sitter/tree-sitter-c-sharp" "master" "src"))
>       treesit-load-name-override-list '((c-sharp
> "libtree-sitter-csharp" "tree_sitter_c_sharp"))
>       major-mode-remap-alist '((csharp-mode . csharp-ts-mode)))
>
> Insert the following text into a csharp-ts-mode buffer:
>
> if (true)
> var x = 2;
>
> Try to indent the variable declaration of the function with
> indent-for-tab-command. Nothing will happen. I'd expect to see this:
>
> if (true)
>     var x = 2;
>
> This issue can be fixed with the following patch:
>
> diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
> index 53c52e6..1a7d535 100644
> --- a/lisp/progmodes/csharp-mode.el
> +++ b/lisp/progmodes/csharp-mode.el
> @@ -678,7 +678,8 @@ csharp-ts-mode--indent-rules
>       ((parent-is "binary_expression") parent 0)
>       ((parent-is "block") parent-bol csharp-ts-mode-indent-offset)
>       ((parent-is "local_function_statement") parent-bol 0)
> -     ((parent-is "if_statement") parent-bol 0)
> +     ((match "block" "if_statement") parent-bol 0)
> +     ((parent-is "if_statement") parent-bol csharp-ts-mode-indent-offset)
>       ((parent-is "for_statement") parent-bol 0)
>       ((parent-is "for_each_statement") parent-bol 0)
>       ((parent-is "while_statement") parent-bol 0)
>

Looks good to me. Are you willing to pack this up with a nice test
confirming the behavior?

All the best,
Theo





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-15 19:01 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-04-15 21:38   ` Jacob Leeming
  2024-04-16  0:27     ` Dmitry Gutov
  2024-04-22  8:50   ` Jacob Leeming
  1 sibling, 1 reply; 15+ messages in thread
From: Jacob Leeming @ 2024-04-15 21:38 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: 70345

Theodor Thornhill <theo@thornhill.no> writes:

>> Hi all,
>>
>
> Hi, Jacob!
>
>>From emacs -Q:
>>
>> Evaluate this elisp to set up treesitter for csharp:
>>
>> (setq treesit-language-source-alist '((c-sharp
>> "https://github.com/tree-sitter/tree-sitter-c-sharp" "master" "src"))
>>       treesit-load-name-override-list '((c-sharp
>> "libtree-sitter-csharp" "tree_sitter_c_sharp"))
>>       major-mode-remap-alist '((csharp-mode . csharp-ts-mode)))
>>
>> Insert the following text into a csharp-ts-mode buffer:
>>
>> if (true)
>> var x = 2;
>>
>> Try to indent the variable declaration of the function with
>> indent-for-tab-command. Nothing will happen. I'd expect to see this:
>>
>> if (true)
>>     var x = 2;
>>
>> This issue can be fixed with the following patch:
>>
>> diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
>> index 53c52e6..1a7d535 100644
>> --- a/lisp/progmodes/csharp-mode.el
>> +++ b/lisp/progmodes/csharp-mode.el
>> @@ -678,7 +678,8 @@ csharp-ts-mode--indent-rules
>>       ((parent-is "binary_expression") parent 0)
>>       ((parent-is "block") parent-bol csharp-ts-mode-indent-offset)
>>       ((parent-is "local_function_statement") parent-bol 0)
>> -     ((parent-is "if_statement") parent-bol 0)
>> +     ((match "block" "if_statement") parent-bol 0)
>> +     ((parent-is "if_statement") parent-bol csharp-ts-mode-indent-offset)
>>       ((parent-is "for_statement") parent-bol 0)
>>       ((parent-is "for_each_statement") parent-bol 0)
>>       ((parent-is "while_statement") parent-bol 0)
>>
>
> Looks good to me. Are you willing to pack this up with a nice test
> confirming the behavior?
>
> All the best,
> Theo

I'm willing, where can I find other tests for csharp-ts-mode? Had a
quick look round the emacs repo but could not find any.

Cheers,
Jacob





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-15 21:38   ` Jacob Leeming
@ 2024-04-16  0:27     ` Dmitry Gutov
  0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Gutov @ 2024-04-16  0:27 UTC (permalink / raw)
  To: Jacob Leeming, Theodor Thornhill; +Cc: 70345

On 16/04/2024 00:38, Jacob Leeming wrote:
> I'm willing, where can I find other tests for csharp-ts-mode? Had a
> quick look round the emacs repo but could not find any.

Check out

test/lisp/progmodes/csharp-mode-resources/indent.erts
and
test/lisp/progmodes/csharp-mode-tests.el

These are for csharp-mode (non tree-sitter version), so you'll basically 
need to make a copy of the latter to run the counterpart for the former.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-15 19:01 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-04-15 21:38   ` Jacob Leeming
@ 2024-04-22  8:50   ` Jacob Leeming
  2024-04-25 16:04     ` Eli Zaretskii
  1 sibling, 1 reply; 15+ messages in thread
From: Jacob Leeming @ 2024-04-22  8:50 UTC (permalink / raw)
  To: Theodor Thornhill, Dmitry Gutov; +Cc: 70345, Jacob Leeming

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

> Theodor Thornhill <theo@thornhill.no> writes:

>> Hi all,
>>
>
> Hi, Jacob!
>
>>>From emacs -Q:
>>
>> Evaluate this elisp to set up treesitter for csharp:
>>
>> (setq treesit-language-source-alist '((c-sharp
>> "https://github.com/tree-sitter/tree-sitter-c-sharp" "master" "src"))
>>       treesit-load-name-override-list '((c-sharp
>> "libtree-sitter-csharp" "tree_sitter_c_sharp"))
>>       major-mode-remap-alist '((csharp-mode . csharp-ts-mode)))
>>
>> Insert the following text into a csharp-ts-mode buffer:
>>
>> if (true)
>> var x = 2;
>>
>> Try to indent the variable declaration of the function with
>> indent-for-tab-command. Nothing will happen. I'd expect to see this:
>>
>> if (true)
>>     var x = 2;
>>
>> This issue can be fixed with the following patch:
>>
>> diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
>> index 53c52e6..1a7d535 100644
>> --- a/lisp/progmodes/csharp-mode.el
>> +++ b/lisp/progmodes/csharp-mode.el
>> @@ -678,7 +678,8 @@ csharp-ts-mode--indent-rules
>>       ((parent-is "binary_expression") parent 0)
>>       ((parent-is "block") parent-bol csharp-ts-mode-indent-offset)
>>       ((parent-is "local_function_statement") parent-bol 0)
>> -     ((parent-is "if_statement") parent-bol 0)
>> +     ((match "block" "if_statement") parent-bol 0)
>> +     ((parent-is "if_statement") parent-bol csharp-ts-mode-indent-offset)
>>       ((parent-is "for_statement") parent-bol 0)
>>       ((parent-is "for_each_statement") parent-bol 0)
>>       ((parent-is "while_statement") parent-bol 0)
>>
>
> Looks good to me. Are you willing to pack this up with a nice test
> confirming the behavior?
>
> All the best,
> Theo

Thanks all,

Discovered we had a similar issue for else blocks. Wrote a test that
covers both cases.

See the attached diff which contains my changes to the indent rules and
the test.

Cheers,
Jacob


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-indent-single-statement-if-in-csharp-ts-mode.patch --]
[-- Type: text/x-diff, Size: 3486 bytes --]

From cd86e2e3fbe596b999f4cfd9655f0a37ac3845d2 Mon Sep 17 00:00:00 2001
From: Jacob Leeming <jacobtophatleeming@gmail.com>
Date: Mon, 22 Apr 2024 09:49:15 +0100
Subject: [PATCH] indent single statement if in csharp-ts-mode

bug#70345
---
 lisp/progmodes/csharp-mode.el                 |  4 +-
 .../csharp-ts-mode-resources/indent.erts      | 51 +++++++++++++++++++
 test/lisp/progmodes/csharp-ts-mode-tests.el   | 30 +++++++++++
 3 files changed, 84 insertions(+), 1 deletion(-)
 create mode 100644 test/lisp/progmodes/csharp-ts-mode-resources/indent.erts
 create mode 100644 test/lisp/progmodes/csharp-ts-mode-tests.el

diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
index 607360f737a..62bbbfe02ff 100644
--- a/lisp/progmodes/csharp-mode.el
+++ b/lisp/progmodes/csharp-mode.el
@@ -689,7 +689,9 @@ csharp-ts-mode--indent-rules
      ((parent-is "binary_expression") parent 0)
      ((parent-is "block") parent-bol csharp-ts-mode-indent-offset)
      ((parent-is "local_function_statement") parent-bol 0)
-     ((parent-is "if_statement") parent-bol 0)
+     ((match "block" "if_statement") parent-bol 0)
+     ((match "else" "if_statement") parent-bol 0)
+     ((parent-is "if_statement") parent-bol csharp-ts-mode-indent-offset)
      ((parent-is "for_statement") parent-bol 0)
      ((parent-is "for_each_statement") parent-bol 0)
      ((parent-is "while_statement") parent-bol 0)
diff --git a/test/lisp/progmodes/csharp-ts-mode-resources/indent.erts b/test/lisp/progmodes/csharp-ts-mode-resources/indent.erts
new file mode 100644
index 00000000000..3cb23608270
--- /dev/null
+++ b/test/lisp/progmodes/csharp-ts-mode-resources/indent.erts
@@ -0,0 +1,51 @@
+Code:
+  (lambda ()
+    (csharp-ts-mode)
+    (indent-region (point-min) (point-max)))
+
+Point-Char: |
+
+Name: Indent single statement body for if/else. (bug#70345)
+
+=-=
+
+int x;
+int y;
+
+if (true)
+    x = 2;
+
+if (true)
+{
+    x = 2;
+}
+
+if (true)
+    x = 2;
+else
+    y = 2;
+
+if (true)
+{
+    x = 2;
+}
+else
+{
+    y = 2;
+}
+
+if (true)
+    x = 2;
+else
+{
+    y = 2;
+}
+
+if (true)
+{
+    x = 2;
+}
+else
+    y = 2;
+
+=-=-=
diff --git a/test/lisp/progmodes/csharp-ts-mode-tests.el b/test/lisp/progmodes/csharp-ts-mode-tests.el
new file mode 100644
index 00000000000..0df0211d86b
--- /dev/null
+++ b/test/lisp/progmodes/csharp-ts-mode-tests.el
@@ -0,0 +1,30 @@
+;;; csharp-ts-mode-tests.el --- Tests for Tree-sitter-based C# mode  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2024 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'csharp-mode)
+
+(ert-deftest csharp-ts-mode-test-indentation ()
+  (ert-test-erts-file (ert-resource-file "indent.erts")))
+
+(provide 'csharp-ts-mode-tests)
+;;; csharp-ts-mode-tests.el ends here
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-22  8:50   ` Jacob Leeming
@ 2024-04-25 16:04     ` Eli Zaretskii
  2024-04-26 13:53       ` Jacob Leeming
  2024-04-27 13:10       ` john muhl
  0 siblings, 2 replies; 15+ messages in thread
From: Eli Zaretskii @ 2024-04-25 16:04 UTC (permalink / raw)
  To: Jacob Leeming; +Cc: dmitry, 70345-done, theo

> Cc: 70345@debbugs.gnu.org, Jacob Leeming <jacobtophatleeming@gmail.com>
> From: Jacob Leeming <jacobtophatleeming@gmail.com>
> Date: Mon, 22 Apr 2024 09:50:23 +0100
> 
> > Looks good to me. Are you willing to pack this up with a nice test
> > confirming the behavior?
> >
> > All the best,
> > Theo
> 
> Thanks all,
> 
> Discovered we had a similar issue for else blocks. Wrote a test that
> covers both cases.
> 
> See the attached diff which contains my changes to the indent rules and
> the test.

Thanks, I installed this on the emacs-29 branch.  (The test you added
should have been added to csharp-mode-tests.el, since our test files
follow the names of the implementation files, and csharp-ts-mode is
implemented in csharp-mode.el.  I fixed that.)

With this changeset you have exhausted the amount of changes that we
can accept from you without copyright assignment.  Would you like to
start the paperwork of assigning the copyright at this time, so that
we could accept your contributions in the future without limitations?
If yes, I will send you the form to fill and the instructions to send
the form.

I'm closing this bug.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-25 16:04     ` Eli Zaretskii
@ 2024-04-26 13:53       ` Jacob Leeming
  2024-04-26 15:24         ` Eli Zaretskii
  2024-04-27 13:10       ` john muhl
  1 sibling, 1 reply; 15+ messages in thread
From: Jacob Leeming @ 2024-04-26 13:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70345

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 70345@debbugs.gnu.org, Jacob Leeming <jacobtophatleeming@gmail.com>
>> From: Jacob Leeming <jacobtophatleeming@gmail.com>
>> Date: Mon, 22 Apr 2024 09:50:23 +0100
>> 
>> > Looks good to me. Are you willing to pack this up with a nice test
>> > confirming the behavior?
>> >
>> > All the best,
>> > Theo
>> 
>> Thanks all,
>> 
>> Discovered we had a similar issue for else blocks. Wrote a test that
>> covers both cases.
>> 
>> See the attached diff which contains my changes to the indent rules and
>> the test.
>
> Thanks, I installed this on the emacs-29 branch.  (The test you added
> should have been added to csharp-mode-tests.el, since our test files
> follow the names of the implementation files, and csharp-ts-mode is
> implemented in csharp-mode.el.  I fixed that.)
>
> With this changeset you have exhausted the amount of changes that we
> can accept from you without copyright assignment.  Would you like to
> start the paperwork of assigning the copyright at this time, so that
> we could accept your contributions in the future without limitations?
> If yes, I will send you the form to fill and the instructions to send
> the form.
>
> I'm closing this bug.

Thanks Eli,

Please send the paperwork! I'll get it filled in and sent off when
possible. If I have future questions about the paperwork, is it best to
use the emacs devel mailing list?

Cheers,
Jacob





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-26 13:53       ` Jacob Leeming
@ 2024-04-26 15:24         ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2024-04-26 15:24 UTC (permalink / raw)
  To: Jacob Leeming; +Cc: 70345

> From: Jacob Leeming <jacobtophatleeming@gmail.com>
> Cc: 70345@debbugs.gnu.org
> Date: Fri, 26 Apr 2024 14:53:36 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > With this changeset you have exhausted the amount of changes that we
> > can accept from you without copyright assignment.  Would you like to
> > start the paperwork of assigning the copyright at this time, so that
> > we could accept your contributions in the future without limitations?
> > If yes, I will send you the form to fill and the instructions to send
> > the form.
> >
> > I'm closing this bug.
> 
> Thanks Eli,
> 
> Please send the paperwork! I'll get it filled in and sent off when
> possible.

Thanks, form sent off-list.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-25 16:04     ` Eli Zaretskii
  2024-04-26 13:53       ` Jacob Leeming
@ 2024-04-27 13:10       ` john muhl
  2024-04-27 15:41         ` Eli Zaretskii
  1 sibling, 1 reply; 15+ messages in thread
From: john muhl @ 2024-04-27 13:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dmitry, 70345, theo, Jacob Leeming

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 70345@debbugs.gnu.org, Jacob Leeming <jacobtophatleeming@gmail.com>
>> From: Jacob Leeming <jacobtophatleeming@gmail.com>
>> Date: Mon, 22 Apr 2024 09:50:23 +0100
>> 
>> > Looks good to me. Are you willing to pack this up with a nice test
>> > confirming the behavior?
>> >
>> > All the best,
>> > Theo
>> 
>> Thanks all,
>> 
>> Discovered we had a similar issue for else blocks. Wrote a test that
>> covers both cases.
>> 
>> See the attached diff which contains my changes to the indent rules and
>> the test.
>
> Thanks, I installed this on the emacs-29 branch.  (The test you added
> should have been added to csharp-mode-tests.el, since our test files
> follow the names of the implementation files, and csharp-ts-mode is
> implemented in csharp-mode.el.  I fixed that.)

The test should check that the c-sharp grammar is available so
that it gets marked as skipped instead of failed.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Skip-csharp-ts-mode-test-if-grammar-is-missing.patch --]
[-- Type: text/x-patch, Size: 937 bytes --]

From 068cad8612c31cea41f0cc21a865efe0785d4e7a Mon Sep 17 00:00:00 2001
From: john muhl <jm@pub.pink>
Date: Sat, 27 Apr 2024 09:55:42 -0500
Subject: [PATCH] ; Skip 'csharp-ts-mode' test if grammar is missing

* test/lisp/progmodes/csharp-mode-tests.el
(csharp-ts-mode-test-indentation): Skip test instead of failing.
---
 test/lisp/progmodes/csharp-mode-tests.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/lisp/progmodes/csharp-mode-tests.el b/test/lisp/progmodes/csharp-mode-tests.el
index 2878fa601f2..b3c57a7026b 100644
--- a/test/lisp/progmodes/csharp-mode-tests.el
+++ b/test/lisp/progmodes/csharp-mode-tests.el
@@ -27,6 +27,7 @@ csharp-mode-test-indentation
   (ert-test-erts-file (ert-resource-file "indent.erts")))
 
 (ert-deftest csharp-ts-mode-test-indentation ()
+  (skip-unless (treesit-ready-p 'c-sharp))
   (ert-test-erts-file (ert-resource-file "indent-ts.erts")))
 
 (provide 'csharp-mode-tests)
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-27 13:10       ` john muhl
@ 2024-04-27 15:41         ` Eli Zaretskii
  2024-04-27 17:13           ` john muhl
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2024-04-27 15:41 UTC (permalink / raw)
  To: john muhl; +Cc: dmitry, 70345, theo, jacobtophatleeming

> From: john muhl <jm@pub.pink>
> Cc: Jacob Leeming <jacobtophatleeming@gmail.com>, dmitry@gutov.dev,
>  70345@debbugs.gnu.org, theo@thornhill.no
> Date: Sat, 27 Apr 2024 08:10:47 -0500
> 
> The test should check that the c-sharp grammar is available so
> that it gets marked as skipped instead of failed.
> 
> >From 068cad8612c31cea41f0cc21a865efe0785d4e7a Mon Sep 17 00:00:00 2001
> From: john muhl <jm@pub.pink>
> Date: Sat, 27 Apr 2024 09:55:42 -0500
> Subject: [PATCH] ; Skip 'csharp-ts-mode' test if grammar is missing
> 
> * test/lisp/progmodes/csharp-mode-tests.el
> (csharp-ts-mode-test-indentation): Skip test instead of failing.
> ---
>  test/lisp/progmodes/csharp-mode-tests.el | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/test/lisp/progmodes/csharp-mode-tests.el b/test/lisp/progmodes/csharp-mode-tests.el
> index 2878fa601f2..b3c57a7026b 100644
> --- a/test/lisp/progmodes/csharp-mode-tests.el
> +++ b/test/lisp/progmodes/csharp-mode-tests.el
> @@ -27,6 +27,7 @@ csharp-mode-test-indentation
>    (ert-test-erts-file (ert-resource-file "indent.erts")))
>  
>  (ert-deftest csharp-ts-mode-test-indentation ()
> +  (skip-unless (treesit-ready-p 'c-sharp))
>    (ert-test-erts-file (ert-resource-file "indent-ts.erts")))

Thanks, but shouldn't we invoke treesit-ready-p with second argument
non-nil?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-27 15:41         ` Eli Zaretskii
@ 2024-04-27 17:13           ` john muhl
  2024-04-27 19:18             ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: john muhl @ 2024-04-27 17:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dmitry, 70345, theo, jacobtophatleeming

Eli Zaretskii <eliz@gnu.org> writes:

>> From: john muhl <jm@pub.pink>
>> Cc: Jacob Leeming <jacobtophatleeming@gmail.com>, dmitry@gutov.dev,
>>  70345@debbugs.gnu.org, theo@thornhill.no
>> Date: Sat, 27 Apr 2024 08:10:47 -0500
>> 
>> The test should check that the c-sharp grammar is available so
>> that it gets marked as skipped instead of failed.
>> 
>> >From 068cad8612c31cea41f0cc21a865efe0785d4e7a Mon Sep 17 00:00:00 2001
>> From: john muhl <jm@pub.pink>
>> Date: Sat, 27 Apr 2024 09:55:42 -0500
>> Subject: [PATCH] ; Skip 'csharp-ts-mode' test if grammar is missing
>> 
>> * test/lisp/progmodes/csharp-mode-tests.el
>> (csharp-ts-mode-test-indentation): Skip test instead of failing.
>> ---
>>  test/lisp/progmodes/csharp-mode-tests.el | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/test/lisp/progmodes/csharp-mode-tests.el b/test/lisp/progmodes/csharp-mode-tests.el
>> index 2878fa601f2..b3c57a7026b 100644
>> --- a/test/lisp/progmodes/csharp-mode-tests.el
>> +++ b/test/lisp/progmodes/csharp-mode-tests.el
>> @@ -27,6 +27,7 @@ csharp-mode-test-indentation
>>    (ert-test-erts-file (ert-resource-file "indent.erts")))
>>  
>>  (ert-deftest csharp-ts-mode-test-indentation ()
>> +  (skip-unless (treesit-ready-p 'c-sharp))
>>    (ert-test-erts-file (ert-resource-file "indent-ts.erts")))
>
> Thanks, but shouldn't we invoke treesit-ready-p with second argument
> non-nil?

Looks like it’s gone either way so far.

$ git grep 'treesit-ready-p' test/lisp/
test/lisp/align-tests.el:(autoload 'treesit-ready-p "treesit")
test/lisp/align-tests.el:  (skip-unless (treesit-ready-p 'lua))
test/lisp/progmodes/c-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'c))
test/lisp/progmodes/c-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'c))
test/lisp/progmodes/c-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'c))
test/lisp/progmodes/c-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'c))
test/lisp/progmodes/csharp-mode-tests.el:  (skip-unless (treesit-ready-p 'c-sharp))
test/lisp/progmodes/elixir-ts-mode-tests.el:  (skip-unless (and (treesit-ready-p 'elixir) (treesit-ready-p 'heex)))
test/lisp/progmodes/go-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'go))
test/lisp/progmodes/heex-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'heex))
test/lisp/progmodes/java-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'java))
test/lisp/progmodes/java-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'java))
test/lisp/progmodes/js-tests.el:  (skip-unless (treesit-ready-p 'javascript))
test/lisp/progmodes/lua-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'lua))
test/lisp/progmodes/lua-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'lua))
test/lisp/progmodes/lua-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'lua))
test/lisp/progmodes/lua-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'lua))
test/lisp/progmodes/python-tests.el:     (skip-unless (treesit-ready-p 'python))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/ruby-ts-mode-tests.el:     (skip-unless (treesit-ready-p 'ruby t))
test/lisp/progmodes/rust-ts-mode-tests.el:  (skip-unless (treesit-ready-p 'rust))
test/lisp/progmodes/typescript-ts-mode-tests.el:  (skip-unless (and (treesit-ready-p 'typescript)
test/lisp/progmodes/typescript-ts-mode-tests.el:                    (treesit-ready-p 'tsx)))






^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body
  2024-04-27 17:13           ` john muhl
@ 2024-04-27 19:18             ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2024-04-27 19:18 UTC (permalink / raw)
  To: john muhl; +Cc: dmitry, 70345, theo, jacobtophatleeming

> From: john muhl <jm@pub.pink>
> Cc: jacobtophatleeming@gmail.com, dmitry@gutov.dev, 70345@debbugs.gnu.org,
>  theo@thornhill.no
> Date: Sat, 27 Apr 2024 12:13:12 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, but shouldn't we invoke treesit-ready-p with second argument
> > non-nil?
> 
> Looks like it’s gone either way so far.

"Two wrongs don't make a right."  Without the 2nd arg, the function
complains:

  Warning (treesit): Cannot activate tree-sitter, because language grammar for c-sharp is unavailable (not-found): (libtree-sitter-c-sharp libtree-sitter-c-sharp.dll) No such file or directory
    skipped  2/2  csharp-ts-mode-test-indentation (0.007771 sec)

So I've added the 2nd argument to your patch and installed it on the
emacs-29 branch.

Thanks.





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-04-27 19:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-11 20:32 bug#70345: [PATCH] 29.1.50; csharp-ts-mode indentation of if statements with single-statement body Jacob Leeming
2024-04-12  5:45 ` Eli Zaretskii
2024-04-14 23:02 ` Yuan Fu
2024-04-15  4:56   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-15 19:01 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-15 21:38   ` Jacob Leeming
2024-04-16  0:27     ` Dmitry Gutov
2024-04-22  8:50   ` Jacob Leeming
2024-04-25 16:04     ` Eli Zaretskii
2024-04-26 13:53       ` Jacob Leeming
2024-04-26 15:24         ` Eli Zaretskii
2024-04-27 13:10       ` john muhl
2024-04-27 15:41         ` Eli Zaretskii
2024-04-27 17:13           ` john muhl
2024-04-27 19:18             ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).