bug#43559: 28.0.50; [PATCH] Add csharp support to cc-mode

["Jostein Kjønigsen" <jostein@secure.kjonigsen.net>]

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

On 22.09.2020 15:10, Theodor Thornhill wrote:
> Jostein Kjønigsen <jostein@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@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no

[-- Attachment #2.1: Type: text/html, Size: 7246 bytes --]

[-- Attachment #2.2: nhebmpbbonkfichd.png --]
[-- Type: image/png, Size: 7374 bytes --]

[-- Attachment #2.3: epihfhegglofbbha.png --]
[-- Type: image/png, Size: 7013 bytes --]

[-- Attachment #2.4: hfhhbebmljemcgca.png --]
[-- Type: image/png, Size: 5608 bytes --]

[-- Attachment #2.5: fiaafecacbkbgbkh.png --]
[-- Type: image/png, Size: 5475 bytes --]

[-- Attachment #2.6: aceilhdglgpjgcad.png --]
[-- Type: image/png, Size: 5913 bytes --]

[-- Attachment #2.7: mehgknkpcccjilmm.png --]
[-- Type: image/png, Size: 6620 bytes --]

[-- Attachment #2.8: jognflfdjkamkonl.png --]
[-- Type: image/png, Size: 6622 bytes --]

[-- Attachment #2.9: igcegelpjofikpgg.png --]
[-- Type: image/png, Size: 19080 bytes --]