GNU bug report logs - #43559
28.0.50; [PATCH] Add csharp support to cc-mode

Previous Next

Package: emacs;

Reported by: Theodor Thornhill <theo <at> thornhill.no>

Date: Tue, 22 Sep 2020 10:39:02 UTC

Severity: normal

Found in version 28.0.50

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43559 in the body.
You can then email your comments to 43559 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Tue, 22 Sep 2020 10:39:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Theodor Thornhill <theo <at> thornhill.no>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 22 Sep 2020 10:39:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: bug-gnu-emacs <at> gnu.org
Cc: acm <at> muc.de, jostein <at> kjonigsen.net
Subject: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Tue, 22 Sep 2020 12:37:54 +0200
[Message part 1 (text/plain, inline)]

Hello!

For some time, I've been a little dissatisfied with the support for C#
in emacs.  Recently, there has been some breakage in handling of strings
and fontification, more precisely:

  - https://github.com/josteink/csharp-mode/issues/162

Also, it is using a lot of internal c-mode functions:
  - https://github.com/josteink/csharp-mode/issues/162

I've had some discussions with with the current maintainer, Jostein
Kjønigsen (cc'd) about the future of this mode in
https://github.com/josteink/csharp-mode/issues/162.

Some points of note:
  - It cannot easily be donated to emacs due to paperwork
  - Jostein does not have time to do it himself
  - I took upon myself to rewrite it in a separate branch, as an end
  goal for it to be included in emacs

While working on this, I realized the easiest way to get something good
here is to overall reduce the complexity.  As such I created this patch.

It seems to work nicely, removing the need for the external mode
completely.

However, I see that including csharp here is most likely a bigger effort
than just sending this patch, but I wanted to send what I have, and
hopefully get some pointers.  Or in worst case stop before I go too
far..

This patch is written without consulting the current mode, so paperwork
should not be an issue.

In addition, the current mode has an implementation of imenu rewritten
from scratch by Jostein, and as such I think this can be included as
well.

I tried not to do too much, since I believe most of the advanced
functionality should be provided by an lsp mode such as eglot or lsp-mode.

Have a nice day!
Theodor Thornhill

[0001-Add-csharp-support-to-cc-mode.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Tue, 22 Sep 2020 12:52:01 GMT) Full text and rfc822 format available.

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Theodor Thornhill <theo <at> thornhill.no>, bug-gnu-emacs <at> gnu.org
Cc: acm <at> muc.de, jostein <at> kjonigsen.net
Subject: Re: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Tue, 22 Sep 2020 14:51:30 +0200
[Message part 1 (text/plain, inline)]
Hey Theo.

Thanks for your work on trying to get something new, and possibly better 
C#-support mainlined in Emacs!

Not commenting on the patch itself, but in your email you had some links 
to Github issues which were not correct.

To assist the further conversation, I'll just provide pointers to the 
actual issues below.


On 22.09.2020 12:37, Theodor Thornhill wrote:
> Also, it is using a lot of internal c-mode functions:
>    - https://github.com/josteink/csharp-mode/issues/162

This link should be: https://github.com/josteink/csharp-mode/issues/128

>
> I've had some discussions with with the current maintainer, Jostein
> Kjønigsen (cc'd) about the future of this mode in
> https://github.com/josteink/csharp-mode/issues/162.

This link should be: https://github.com/josteink/csharp-mode/issues/153

Hope this helps.

-- 

Kind regards
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Tue, 22 Sep 2020 13:11:02 GMT) Full text and rfc822 format available.

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>,
 bug-gnu-emacs <at> gnu.org
Cc: acm <at> muc.de
Subject: Re: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Tue, 22 Sep 2020 15:10:10 +0200
Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> writes:

> Hey Theo.
>
> Thanks for your work on trying to get something new, and possibly better 
> C#-support mainlined in Emacs!
My pleasure - hopefully we'll get there :)

[...]

> On 22.09.2020 12:37, Theodor Thornhill wrote:
>> Also, it is using a lot of internal c-mode functions:
>>    - https://github.com/josteink/csharp-mode/issues/162
>
> This link should be: https://github.com/josteink/csharp-mode/issues/128
>
>>
>> I've had some discussions with with the current maintainer, Jostein
>> Kjønigsen (cc'd) about the future of this mode in
>> https://github.com/josteink/csharp-mode/issues/162.
>
> This link should be: https://github.com/josteink/csharp-mode/issues/153
>

When you think you can copy paste... Thanks for correcting here.  Don't
know how I managed that :)

Theo




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Tue, 22 Sep 2020 14:27:01 GMT) Full text and rfc822 format available.

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Theodor Thornhill <theo <at> thornhill.no>, bug-gnu-emacs <at> gnu.org
Cc: acm <at> muc.de
Subject: Re: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Tue, 22 Sep 2020 16:26:41 +0200
[Message part 1 (text/plain, inline)]
On 22.09.2020 15:10, Theodor Thornhill wrote:
> Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> writes:
>
>> Hey Theo.
>>
>> Thanks for your work on trying to get something new, and possibly better
>> C#-support mainlined in Emacs!
> My pleasure - hopefully we'll get there :)
>
> [...]
>
Hey Theo.

I still haven't looked at the code in your patch, but I've applied it to 
latest Emacs Git master, and given it a test-run.

When going over existing code, everything looks and feels nice. And it's 
much faster than my "old", non-transferable code. So I definitely think 
this is a /very good/ start.

While I hold no "position" in GNU or Emacs as a whole, I would consider 
myself a C# veteran (since 2004), and as the old maintainer of old 
csharp-mode, I think I have some experience which may be useful.

So for the time being, I'll just promote myself to being the "critical 
Emacs C#-user", and provide some feedback based on my expectations for 
how editing C# should feel or behave.

Please don't consider the following comments as negative towards your 
work, or me in any way trying to dissuade you from getting this 
mainlined. I'm just here to try to help you ensure that what you land is 
going to be "good enough" so that it will be easier to argue for 
inclusion in core-Emacs later on :)

So with all that said, so far I've noticed a few things which I think 
arguably goes against C# convention, and would be nice to get fixed, if 
possible. There's also a some things where I just feel it would "better" 
to do things differently.

Roughly it boils down to this list:

1. Attributes

2. Object initializers

3. Lambda indentation

4. Misc variables/field fontification issues


*First off: **Attributes are not handled properly. **
*

In java-mode annotations gets fontified and indented properly:

In your current csharp-mode draft, we get no fontification, and we also 
get a trailing indentation bug for the equivalent C# code:

For annotation-heavy frameworks like ASP.Net Core this might get 
annoying. It would be great if this could get fixed.


*Second: Object initializers are not indented properly.*

Consider the following fairly simple case. Using the provided patch we 
get the following indentation:

According to C# convention, one would expect something more like this:

This one is arguably very hard to get right, because it's a conceptually 
infinitely recursive problem. (You may initialize a property with 
another object-initializer.)

While solving this perfectly for all cases is clearly out the window, do 
you think it would be possible at least to make 1-level 
object-initilizers work?

One the bright side /collection-/initializers seems to work just fine :)


*Third: Lamda-function indentation when used with higher order functions
*

Lambdas also suffer from some unexpected indentation issues:

Here I would expect the following indentation instead:


*Fourth: variable-fontification*

Here I have no absolute C# convention to quote for absolute correctness, 
but it kinda "feels wrong" to me at places.

Without a proper language-engine to guide us, it will be literally 
impossible to know what is a class, a namespace, an enum, a local field 
or an actual variable at any point in the code, and fontify the code 
100% correctly based on that.

So we'll have to make due with imperfections, make some pragmatical 
decisions on what we think will be good default/fallback values, and 
that's OK.

Right now though, all implicitly typed variables (vars), local variables 
with method-access and local fields with property-access are shown using 
/font-lock-constant-face/ and that seems a bit off:

For var-heavy functions, this inconsistency is somewhat distracting, and 
I would much prefer a font-lock-variable-face fallback in these cases.

As for "_field" and "foo.", I'm not sure what the best fallback would 
be. Without a language-engine to guide us, this is genuinely hard stuff 
to get right.

By C# convention pascalCased or _underCased identifiers typically 
represent local variables and object-local fields (and thus might use 
font-lock-variable-face as a fallback).

Conversely CamelCased identifiers typically represent either Classes or 
public properties/methods on those, depending on identifier in question 
being dot-accessed or not (and for these another face might make more 
sense).

I'm sure just/discussing/ the///soft-rules/ for these aspects could take 
months alone, not speaking for the effort required to actually making it 
happen in code.

Consider it something to aim for, but I wouldn't expect anyone to get 
this right in a version 1 of anything.


Otherwise, great work, and thank you for putting in this effort!

-- 
Kind regards
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no
[Message part 2 (text/html, inline)]
[nhebmpbbonkfichd.png (image/png, inline)]
[epihfhegglofbbha.png (image/png, inline)]
[hfhhbebmljemcgca.png (image/png, inline)]
[fiaafecacbkbgbkh.png (image/png, inline)]
[aceilhdglgpjgcad.png (image/png, inline)]
[mehgknkpcccjilmm.png (image/png, inline)]
[jognflfdjkamkonl.png (image/png, inline)]
[igcegelpjofikpgg.png (image/png, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Tue, 22 Sep 2020 21:16:02 GMT) Full text and rfc822 format available.

Message #17 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>,
 bug-gnu-emacs <at> gnu.org
Cc: acm <at> muc.de
Subject: Re: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Tue, 22 Sep 2020 23:15:04 +0200
[Message part 1 (text/plain, inline)]
Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> writes:

> On 22.09.2020 15:10, Theodor Thornhill wrote:
>> Jostein Kjønigsen <jostein <at> secure.kjonigsen.net> writes:

[...]

> I still haven't looked at the code in your patch, but I've applied it to 
> latest Emacs Git master, and given it a test-run.
> When going over existing code, everything looks and feels nice. And it's 
> much faster than my "old", non-transferable code. So I definitely think 
> this is a /very good/ start.

Nice!

>
> While I hold no "position" in GNU or Emacs as a whole, I would consider 
> myself a C# veteran (since 2004), and as the old maintainer of old 
> csharp-mode, I think I have some experience which may be useful.
Most definitively.  Thank you for taking your time!

>
> So for the time being, I'll just promote myself to being the "critical 
> Emacs C#-user", and provide some feedback based on my expectations for 
> how editing C# should feel or behave.
>
> Please don't consider the following comments as negative towards your 
> work, or me in any way trying to dissuade you from getting this 
> mainlined. I'm just here to try to help you ensure that what you land is 
> going to be "good enough" so that it will be easier to argue for 
> inclusion in core-Emacs later on :)
None of your comment are taken as negative.  I'd much rather you'd be
critical early, so that we can get something nice quicker!

>
> So with all that said, so far I've noticed a few things which I think 
> arguably goes against C# convention, and would be nice to get fixed, if 
> possible. There's also a some things where I just feel it would "better" 
> to do things differently.
>

Before I answer your comments, I'll just let you know I'm still wrapping
my head around the whole cc-mode.  I might find better solutions to all
this later on.

>
> *First off: **Attributes are not handled properly. **
> *
>
> In java-mode annotations gets fontified and indented properly:
>
> In your current csharp-mode draft, we get no fontification, and we also 
> get a trailing indentation bug for the equivalent C# code:

Yeah, this is a difficult one, because the java version of this
implements lots of logic in the cc-engine.el, and for now, we don't.
However, my most recent patch seems to deal with this properly, as far
as I can tell.  I've tested with your example code and also with random
files in the roslyn repository.  Fontification is still sparse here.

>
> *Second: Object initializers are not indented properly.*
>
> Consider the following fairly simple case. Using the provided patch we 
> get the following indentation:

This one should be fixed in the attached patch.

> This one is arguably very hard to get right, because it's a conceptually 
> infinitely recursive problem. (You may initialize a property with 
> another object-initializer.)

I think the recursion case works as well.  I've made an attempt in the
attached 'test.cs'

>
> While solving this perfectly for all cases is clearly out the window, do 
> you think it would be possible at least to make 1-level 
> object-initilizers work?

Hopefully, we have it now!

>
> One the bright side /collection-/initializers seems to work just fine :)
>

Great!

>
> *Third: Lamda-function indentation when used with higher order functions
> *
>
> Lambdas also suffer from some unexpected indentation issues:

This one should be fixed in the latest patch.

>
> *Fourth: variable-fontification*
>
> Here I have no absolute C# convention to quote for absolute correctness, 
> but it kinda "feels wrong" to me at places.

> So we'll have to make due with imperfections, make some pragmatical 
> decisions on what we think will be good default/fallback values, and 
> that's OK.
>

Agreed

> Right now though, all implicitly typed variables (vars), local variables 
> with method-access and local fields with property-access are shown using 
> /font-lock-constant-face/ and that seems a bit off:

This case is fixed now.  It was due to the 'var' keyword was put in the
wrong basket.

> As for "_field" and "foo.", I'm not sure what the best fallback would 
> be. Without a language-engine to guide us, this is genuinely hard stuff 
> to get right.
>

Yeah, this one is kind of hard, so I've been ignoring it for a little
while.  The easiest thing should be to remove the highlighting, but not
sure if that is the best move so far.

>
> I'm sure just/discussing/ the///soft-rules/ for these aspects could take 
> months alone, not speaking for the effort required to actually making it 
> happen in code.
>
> Consider it something to aim for, but I wouldn't expect anyone to get 
> this right in a version 1 of anything.
>

Yeah, more cases are coming up all the time.  My initial focus has been
mostly on the indentation part, since font-locking is a 'bonus-feature'
in some sense.  However, they seem intimately connected in cc-mode.

>
> Otherwise, great work, and thank you for putting in this effort!

My pleasure :)


One issue I have now is that there are many rules in the c-style-alist,
and I think it would be better to design or reuse some constructs from
cc-engine to apply some other syntax rules to the code.  One example and
consequence of this is the fix of the attributes from your code.  It
aligns nicely, but destroys the indentation of the 'aligned ternary' (on
the bottom of the file).


Please, if you have time, take a look at the second patch, as well as
the test file.  Maybe you get some ideas, but I think some of the low
hanging fruits are found, and it is time to dive into the engine itself.

I really appreciate you taking your time to find these bugs for me.
Thanks.

--
Theo


P.S
Pasting the contents of test.cs here, since I apparently suck at email

/// Working

[Test]
public class Foo
{
    [Zoo]
    [Goo]
    public void Bar()
    {
        
    }
};

var x = new SomeType[]
{
    Prop1 = "foo",
    Prop2 = new SomeType2[]
    {
        Prop3 = true ? "this" : "that";
    }
    //
};


var x = new SomeType[] {
    new SomeType(a, b),
    new SomeType(b, c)
    // lol
};

/// Kind of working

goo.Select(i => {
    return goo.Select(i)
    .Foo()
    .Bar()
});

Foo foo = new Foo();
var bar1 = foo.Bar(_field);
Bar bar3 = foo.Bar(_field.Prop);

/// Not working

var x = foo
? bar
: baz;


[v2-0001-Add-csharp-support-to-cc-mode.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 23 Sep 2020 08:48:01 GMT) Full text and rfc822 format available.

Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: Theodor Thornhill <theo <at> thornhill.no>, bug-gnu-emacs <at> gnu.org
Cc: acm <at> muc.de
Subject: Re: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 23 Sep 2020 10:46:54 +0200
[Message part 1 (text/plain, inline)]
On 22.09.2020 23:15, Theodor Thornhill wrote:
>
> my most recent patch seems to deal with this (attributes) properly, as far
> as I can tell.  I've tested with your example code and also with random
> files in the roslyn repository.  Fontification is still sparse here.
Basic attribute-indentation is confirmed fixed on my end. Still no 
fontification, but I don't think that's critical.
>
>> *Second: Object initializers are not indented properly.*
> This one should be fixed in the attached patch.

It's better, but not quite there. For a simple 1-level nested 
object-initializer, the line containing the closing bracket is still 
incorrectly indented:

>
> I think the recursion case works as well.  I've made an attempt in the
> attached 'test.cs'
I can't reproduce this. Are you sure the attached patch contains all 
your changes?

Looking at your example, your sub-initializer is actually a /collection 
/initializer, which may explain why it seemingly worked on your end.


>
> This (lambdas) one should be fixed in the latest patch.
Confirmed fixed on my end.
>> *Fourth: variable-fontification*
>>
>> Here I have no absolute C# convention to quote for absolute correctness,
>> but it kinda "feels wrong" to me at places.
>> So we'll have to make due with imperfections, make some pragmatical
>> decisions on what we think will be good default/fallback values, and
>> that's OK.
>>
> Agreed
>
>> Right now though, all implicitly typed variables (vars), local variables
>> with method-access and local fields with property-access are shown using
>> /font-lock-constant-face/ and that seems a bit off:
> This case is fixed now.  It was due to the 'var' keyword was put in the
> wrong basket.

Confirmed fixed. This change alone makes things look much better.

In the process of testing this, I've also taken a look at some other 
keywords: const, string, bool, int, async and await.

From what I can tell, they all look proper except for "await".

Looking in the patch, I see it's not mentioned there, so it should 
probably be a quick fix though?

Looking at faces more in depth, I also see annotated functions are not 
getting their function-names fontified.

I guess this goes back to the overall complexity of the attribute-store 
and core cc-mode support?
>
>> As for "_field" and "foo.", I'm not sure what the best fallback would
>> be. Without a language-engine to guide us, this is genuinely hard stuff
>> to get right.
>>
> Yeah, this one is kind of hard, so I've been ignoring it for a little
> while.  The easiest thing should be to remove the highlighting, but not
> sure if that is the best move so far.

I agree this is hard. Even trying to come up with some simple pragmatic 
rules, I'm constantly left thinking about "what ifs"... So we can leave 
it for now.

As for your test-file, I see you've added a multi-line case with a 
ternary operator. That made me look into multi-line statements in 
general. It seems they are not getting indented as expected:

Maybe if you figure out how to fix that, you will also fix the 
ternary-issue at the same time?

-- 
Kind regards
*Jostein Kjønigsen*

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no
[Message part 2 (text/html, inline)]
[cbdccnkapbnidcfb.png (image/png, inline)]
[dadgfepfdgfigdjo.png (image/png, inline)]
[kgdddfodhnglljmd.png (image/png, inline)]
[oclbllokmkogdccb.png (image/png, inline)]
[nhalblonfacnkbej.png (image/png, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 23 Sep 2020 10:13:01 GMT) Full text and rfc822 format available.

Message #23 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 43559 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 23 Sep 2020 10:11:50 +0000
Hello, Theodor.

On Tue, Sep 22, 2020 at 12:37:54 +0200, Theodor Thornhill wrote:


> Hello!

> For some time, I've been a little dissatisfied with the support for C#
> in emacs.  Recently, there has been some breakage in handling of strings
> and fontification, more precisely:

>   - https://github.com/josteink/csharp-mode/issues/128

The current C# mode uses syntax-propertize-function.  This clashes
nastily with CC Mode's use of the syntax-table text property.  When
syntax-propertize-function is non-nil, Emacs arbitrarily wipes all
syntax-table properties from a region then relies on s-p-f to reapply
all the necessary s-t properties.  However the current C# s-p-f only
applies a restricted subset of the necessary properties.  This will
produce random bugs.

The documentation for syntax-propertize-function didn't used to mention
this problem, but it does now.  (As from Emacs 27.2).

The method used in CC Mode to apply and remove these properties is to
add functions, effectively before- or after-change-functions, to either
or both of c-get-state-before-change-functions and
c-before-font-lock-functions.

> Also, it is using a lot of internal c-mode functions:
>   - https://github.com/josteink/csharp-mode/issues/153

> I've had some discussions with with the current maintainer, Jostein
> Kjønigsen (cc'd) about the future of this mode in
> https://github.com/josteink/csharp-mode/issues/162.

> Some points of note:
>   - It cannot easily be donated to emacs due to paperwork
>   - Jostein does not have time to do it himself
>   - I took upon myself to rewrite it in a separate branch, as an end
>   goal for it to be included in emacs

> While working on this, I realized the easiest way to get something good
> here is to overall reduce the complexity.  As such I created this patch.

CC Mode places a high priority on correctness, hence much of its
complexity.  That priority needn't necessarily be carried over to
derived modes.  ;-)

> It seems to work nicely, removing the need for the external mode
> completely.

You're proposing integrating a new C# Mode directly into CC Mode.  This
goes against CC Mode policy, both because it would swell the already
massive code base, and it would increases the danger of "orphan"
submodes.  We already have IDL Mode and Pike Mode which have remained
entirely and almost entirely unchanged in the last 15 years.  The people
who created them are no longer around.

Also, there is no advantage for an integrated mode over a derived mode
separate from CC Mode.  (If there is, that is a bug in CC Mode.  ;-)
There is a slight effort in using c-add-language, that is all.

> However, I see that including csharp here is most likely a bigger effort
> than just sending this patch, but I wanted to send what I have, and
> hopefully get some pointers.  Or in worst case stop before I go too
> far..

I don't see a reason why there shouldn't be a C# Mode directly in Emacs.

> This patch is written without consulting the current mode, so paperwork
> should not be an issue.

> In addition, the current mode has an implementation of imenu rewritten
> from scratch by Jostein, and as such I think this can be included as
> well.

> I tried not to do too much, since I believe most of the advanced
> functionality should be provided by an lsp mode such as eglot or lsp-mode.

That is a completely different topic.  The amount of work needed to use
one of these is massive.  For what it's worth, somebody familiar with
LSPs opined that they weren't yet sufficiently mature to support
something like CC Mode, but this was some while ago (between 1 and 2
years ago).

> Have a nice day!
> Theodor Thornhill

[ Patch snipped, but briefly scanned. ]

-- 
Alan Mackenzie (Nuremberg, Germany).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 23 Sep 2020 18:39:01 GMT) Full text and rfc822 format available.

Message #26 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 43559 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 23 Sep 2020 20:38:22 +0200
Alan Mackenzie <acm <at> muc.de> writes:

> Hello, Theodor.
>

Hello, and thanks for responding!

> On Tue, Sep 22, 2020 at 12:37:54 +0200, Theodor Thornhill wrote:
>
>
>> Hello!
>
>> For some time, I've been a little dissatisfied with the support for C#
>> in emacs.  Recently, there has been some breakage in handling of strings
>> and fontification, more precisely:
>
>>   - https://github.com/josteink/csharp-mode/issues/128
>
> The current C# mode uses syntax-propertize-function.  This clashes
> nastily with CC Mode's use of the syntax-table text property.  When
> syntax-propertize-function is non-nil, Emacs arbitrarily wipes all
> syntax-table properties from a region then relies on s-p-f to reapply
> all the necessary s-t properties.  However the current C# s-p-f only
> applies a restricted subset of the necessary properties.  This will
> produce random bugs.

Thanks for this information.  It will help me investigate further.

>
> The documentation for syntax-propertize-function didn't used to mention
> this problem, but it does now.  (As from Emacs 27.2).
>
> The method used in CC Mode to apply and remove these properties is to
> add functions, effectively before- or after-change-functions, to either
> or both of c-get-state-before-change-functions and
> c-before-font-lock-functions.
>

[...]

>> While working on this, I realized the easiest way to get something good
>> here is to overall reduce the complexity.  As such I created this patch.
>
> CC Mode places a high priority on correctness, hence much of its
> complexity.  That priority needn't necessarily be carried over to
> derived modes.  ;-)
>
Hehe, yeah - it is my hope to leverage more what is already there.

>> It seems to work nicely, removing the need for the external mode
>> completely.
>
> You're proposing integrating a new C# Mode directly into CC Mode.  This
> goes against CC Mode policy, both because it would swell the already
> massive code base, and it would increases the danger of "orphan"
> submodes.  We already have IDL Mode and Pike Mode which have remained
> entirely and almost entirely unchanged in the last 15 years.  The people
> who created them are no longer around.

I see your point here.  I guess I wrongly assumed this should be the
main way to include new cc-mode based modes.

>
> Also, there is no advantage for an integrated mode over a derived mode
> separate from CC Mode.  (If there is, that is a bug in CC Mode.  ;-)
> There is a slight effort in using c-add-language, that is all.
>

That is good to hear.  I guess I'll try to reimplement this patch using
this api, rather than convince you to pollute CC Mode.

>> However, I see that including csharp here is most likely a bigger effort
>> than just sending this patch, but I wanted to send what I have, and
>> hopefully get some pointers.  Or in worst case stop before I go too
>> far..
>
> I don't see a reason why there shouldn't be a C# Mode directly in Emacs.
>
Good. I'll start working on csharp-mode.el, rather than integrating
things.

>> This patch is written without consulting the current mode, so paperwork
>> should not be an issue.
>
>> In addition, the current mode has an implementation of imenu rewritten
>> from scratch by Jostein, and as such I think this can be included as
>> well.
>
>> I tried not to do too much, since I believe most of the advanced
>> functionality should be provided by an lsp mode such as eglot or lsp-mode.
>
> That is a completely different topic.  The amount of work needed to use
> one of these is massive.  For what it's worth, somebody familiar with
> LSPs opined that they weren't yet sufficiently mature to support
> something like CC Mode, but this was some while ago (between 1 and 2
> years ago).

Yeah, I wasn't really suggesting to integrate CC Mode with eglot.  I was
merely commenting on reducing the scope of this potential mode.

Thanks for taking your time so far.  I think I'll for the time being
leave this bug, take it back to github, and create a standalone mode. 

(@Jostein - I'll respond to your comments on GitHub)

Have a nice day/evening,

--
Theodor Thornhill




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Mon, 28 Sep 2020 19:54:02 GMT) Full text and rfc822 format available.

Message #29 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 43559 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Mon, 28 Sep 2020 21:52:00 +0200
Hello, Alan!

Alan Mackenzie <acm <at> muc.de> writes:

[...]

>
> I don't see a reason why there shouldn't be a C# Mode directly in Emacs.
>

The current csharp mode is now rewritten, and I've gotten almost
everything to work nicely apart from one issue, and I was hoping that
you could help me figure it out, or point me in the right direction.

I described it here:
https://lists.gnu.org/archive/html/emacs-devel/2020-09/msg01975.html

However, The lambda case is now fixed, but with a patch to emacs
itself:

--------------------------------------------------------------------------------
diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 4e336c0a06..b415e4b821 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -12011,7 +12011,8 @@ c-looking-at-inexpr-block
 			 (> (point) closest-lim))
 		  (not (bobp))
 		  (progn (backward-char)
-			 (looking-at "[]).]\\|\\w\\|\\s_"))
+			 (or (looking-at "[\]\).]\\|\w\\|\\s_")
+                             (looking-at ">")))
 		  (c-safe (forward-char)
 			  (goto-char (scan-sexps (point) -1))))
 
@@ -12085,7 +12086,11 @@ c-looking-at-inexpr-block
 			  (setq passed-bracket-pairs 1)
 			  (setq bracket-pos (point))))
 		      'maybe)
-		  'maybe))))
+		  'maybe)
+		(if (or (looking-at "([[:alnum:][:space:]_,]*)[ \t\n]*=>[ \t\n]*{")
+			(looking-at "[[:alnum:]_]+[ \t\n]*=>[ \t\n]*{"))
+		    ;; If we are at a C# lambda header
+		    (cons 'inexpr (point))))))
 
       (if (eq res 'maybe)
 	  (cond
--------------------------------------------------------------------------------

This patch lets CC Mode detect the lambda case for C#. I know this is a
patch specific to C#, but I could not find anywhere or anyhow this would
work without these lines.  With them, the indentation in case 1 in the
mentioned link works perfectly.

Maybe this (or something similar) could be eligible for inclusion in CC
Mode, to account for that lambda syntax. I know many other C-like
languages uses similar syntax, so I believe it could be useful for
someone else as well.

Have a nice evening!

--
Theodor Thornhill




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 21 Jul 2021 12:20:02 GMT) Full text and rfc822 format available.

Message #32 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: Alan Mackenzie <acm <at> muc.de>, jostein <at> kjonigsen.net,
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>, 43559 <at> debbugs.gnu.org
Subject: Re: bug#43559: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 21 Jul 2021 14:19:36 +0200
Theodor Thornhill <theo <at> thornhill.no> writes:

>> I don't see a reason why there shouldn't be a C# Mode directly in Emacs.
>
> The current csharp mode is now rewritten,

Great!  Including it in GNU ELPA would be cool.  Stefan?

> However, The lambda case is now fixed, but with a patch to emacs
> itself:
>
> --------------------------------------------------------------------------------
> diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
> index 4e336c0a06..b415e4b821 100644
> --- a/lisp/progmodes/cc-engine.el
> +++ b/lisp/progmodes/cc-engine.el

Alan, would it be OK to include that small change in cc-engine?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Removed tag(s) patch. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Wed, 21 Jul 2021 12:21:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 21 Jul 2021 12:38:02 GMT) Full text and rfc822 format available.

Message #37 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Alan Mackenzie <acm <at> muc.de>, jostein <at> kjonigsen.net,
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>, 43559 <at> debbugs.gnu.org
Subject: Re: bug#43559: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 21 Jul 2021 14:36:41 +0200
[Message part 1 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 21 Jul 2021 12:59:02 GMT) Full text and rfc822 format available.

Message #40 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: Alan Mackenzie <acm <at> muc.de>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 jostein <at> kjonigsen.net, 43559 <at> debbugs.gnu.org
Subject: Re: bug#43559: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 21 Jul 2021 08:58:23 -0400
> > Great!  Including it in GNU ELPA would be cool.  Stefan?

Fine by me.

> Right now it lives in melpa, but there is no particular reason for it
> not to be in elpa/Emacs by default.

Excellent.

> There are some performance issues that are being addressed by Alan
> with regards to multiline strings.

That should have no bearing on inclusion into GNU ELPA.

> The paperwork should be very easy, as I _think_ the only people with
> commits that needs paperwork is Jostein and me.

Can you double check?


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 21 Jul 2021 13:19:02 GMT) Full text and rfc822 format available.

Message #43 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Alan Mackenzie <acm <at> muc.de>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 jostein <at> kjonigsen.net, 43559 <at> debbugs.gnu.org
Subject: Re: bug#43559: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 21 Jul 2021 15:18:02 +0200
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> > Great!  Including it in GNU ELPA would be cool.  Stefan?
>
> Fine by me.
>

Cool!

>> There are some performance issues that are being addressed by Alan
>> with regards to multiline strings.
>
> That should have no bearing on inclusion into GNU ELPA.

No, I assume it should be even easier to change csharp-mode when those
fixes arrive.

>
>> The paperwork should be very easy, as I _think_ the only people with
>> commits that needs paperwork is Jostein and me.
>
> Can you double check?
>

I'm not completely sure how I'd double check that for there to be no
issues at all legally.  When I did the rewrite I did it without looking
at the already existing code, since I wanted the option to include it in
emacs at some point.  If you look at
https://github.com/emacs-csharp/csharp-mode/blame/master/csharp-mode.el
you'll find that most commits are from me, except the header comments,
which I took from the existing code. Most of the other commits that are
referenced in that blame is either whitespace that didn't change when
the new version was merged as well as some one line bug patches that
came after the new version arrived.  I assume they are ok to add to
elpa, given their size?

Other commits are ci related and an indentation fix.  How do I verify
this?

Theo




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 21 Jul 2021 14:07:01 GMT) Full text and rfc822 format available.

Message #46 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Jostein Kjønigsen <jostein <at> secure.kjonigsen.net>
To: "Stefan Monnier" <monnier <at> iro.umontreal.ca>,
 "Theodor Thornhill" <theo <at> thornhill.no>
Cc: Alan Mackenzie <acm <at> muc.de>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Jostein Kjønigsen <jostein <at> kjonigsen.net>,
 43559 <at> debbugs.gnu.org
Subject: Re: bug#43559: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 21 Jul 2021 16:05:53 +0200
[Message part 1 (text/plain, inline)]
On vacation without a laptop, but just chiming in that I’ve previously done the required FSF copyright assignment. 

So any commits on my part should not lead to any additional paperwork. 

--
Vennlig hilsen
Jostein Kjønigsen

jostein <at> kjonigsen.net 🍵 jostein <at> gmail.com
https://jostein.kjønigsen.no <https://jostein.xn--kjnigsen-64a.no/>


On Wed, Jul 21, 2021, at 14:58, Stefan Monnier wrote:
> > > Great!  Including it in GNU ELPA would be cool.  Stefan?
> 
> Fine by me.
> 
> > Right now it lives in melpa, but there is no particular reason for it
> > not to be in elpa/Emacs by default.
> 
> Excellent.
> 
> > There are some performance issues that are being addressed by Alan
> > with regards to multiline strings.
> 
> That should have no bearing on inclusion into GNU ELPA.
> 
> > The paperwork should be very easy, as I _think_ the only people with
> > commits that needs paperwork is Jostein and me.
> 
> Can you double check?
> 
> 
>         Stefan
> 
> 
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#43559; Package emacs. (Wed, 25 Aug 2021 16:09:02 GMT) Full text and rfc822 format available.

Message #49 received at 43559 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 43559 <at> debbugs.gnu.org, Jostein Kjønigsen
 <jostein <at> secure.kjonigsen.net>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Alan Mackenzie <acm <at> muc.de>, jostein <at> kjonigsen.net,
 jose.junior <at> wildlifestudios.com
Subject: Re: bug#43559: 28.0.50; [PATCH] Add csharp support to cc-mode
Date: Wed, 25 Aug 2021 18:07:49 +0200
Theodor Thornhill <theo <at> thornhill.no> writes:

> Seeing how this is how included to elpa I guess this issue can be closed :)
> thanks for your patience!

:-)

I'm closing this issue, then.





bug marked as fixed in version 28.1, send any further explanations to 43559 <at> debbugs.gnu.org and Theodor Thornhill <theo <at> thornhill.no> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Wed, 25 Aug 2021 16:09:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 23 Sep 2021 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 215 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.