'this' extension method parameters aren't fontified

[nosami]

No big deal, but thought I'd log it anyway.

[josteink]

I can definitely reproduce this one.

I do have a pretty clear idea of where this needs to be fixed. Unless someone beats me to the mark with a PR, I will look into this one later when I have time.

Thanks for the report!

[josteink]

Fooling around with the code, it seems they do get fontified if they are regular built-in types, but not custom types.

Stepping through this using font-lock-studio (install package from MELPA), I can see the fontification occurs for built-in types using the following expression:

"\\<\\(\\(?:b\\(?:ool\\|yte\\)\\|char\\|d\\(?:ecimal\\|ouble\\)\\|float\\|int\\|long\\|object\\|s\\(?:byte\\|hort\\|tring\\)\\|u\\(?:int\\|long\\|short\\)\\|void\\)\\)\\>"
  (1 'font-lock-type-face)

Obviously this does not trigger for custom types (which should be fontified font-lock-type-face). Trying to trace this further, we can see that for regular declarations, custom types gets fontified by this expression:

c-font-lock-declarations
  (0 font-lock-keyword-face)

We probably need to extend our fontification to have a csharp-font-lock-declarations which includes whatever is not included by c-font-lock-declarations.

Anyone want to take a stab at this, or should I just put it in the "later, when I have time"-queue? :)

Edit: Following c-font-lock-declarations leads (as expected) straight into cc-mode-internals and cc-fonts.el. I'm very, very, very certain I don't want to submit any patches touching any of this :D

[wasamasa]

Hmm, I've got it working by adding the following to csharp-mode.el:

(c-lang-defconst c-primitive-type-kwds csharp '("this"))

A side effect of it is that if you type this.bar it is fontified as a type until you've entered the first character following the period. Surely more acceptable than the initial issue, no?

[josteink]

How about scenarios where you pass self-references around using this only? Will that fontify it as a type too?

Depending on the style of code you write, I'm almost certain you may encounter that quite often, and I'm not sure making extension-methods work nicely is a good trade-off.

If it isn't hell getting done right, I may consider it, but for now I'd rather add some more fontification-rules to the lot we already have.

[wasamasa]

Please attach sources for testing font-lock, my C#-fu is weak.

[josteink]

See below:

using System;

namespace DemoSpace
{
    public class DemoClass
    {
        private InnerClass _innerClass;

        public DemoClass()
        {
            // note usage of plain "this".
            _innerClass = new InnerClass(this);
        }
    }

    public static class DemoClassExtensions
    {
        public static bool ExampleWithMemberType(this DemoClass instance, string foo)
        {
            // fontifies parameters incorrectly
            return false;
        }

        public static bool ExampleWithBulitInType(this string instance, int bar)
        {
            // fontifies parameters correctly
            return true;
        }
    }
}

Does this properly illustrate the issue?

(Annoyingly, Github DOES not have the bug we're trying to fix :) )

[wasamasa]

Yup, this is fontified as type.

[josteink]

Trying to verify this fix, I cannot reproduce that it fixes method declarations (or does anything at all).

Where in the file did you add this?

[wasamasa]

I did add this at the end, then did M-x eval-buffer and opened a sample file looking like in the screenshot of the initial post.

[josteink]

I added it here, saved the file, restarted emacs and re-evaled csharp-mode from a clean state.

You need to do that for proper testing in my experience. There's just too much eval-on-load thingies, I suspect.

I still get the same issue about extension-methods not being fontified properly (but I do get the issue of this used as a standalone word being fontified incorrectly), so I sadly don't think this fix is sufficient.

Got any other ideas? :)

[theothornhill]

@josteink
This should be fixed in rework, can you confirm? :-)

[josteink]

Yeah works for me too. Once rework is master, this one should be considered closed.