icsharpcode / NRefactory

NRefactory - Refactoring Your C# Code
684 stars 261 forks source link

Make "base" and "this" constructor keywords as identifiers #519

Closed zvirja closed 7 years ago

zvirja commented 7 years ago

Hi guys,

I've found that significant improvement could be applied to the ILSpy. Consider the following code: image

Unfortunately, the base and this words near the constructors are not clickable. I'd expect to have them clickable and navigate to appropriate constructor.

I've found that this is because of the following code in the CSharpOutputVisitor.cs file:

public virtual void VisitConstructorInitializer(ConstructorInitializer constructorInitializer)
{
    //.....
    if (constructorInitializer.ConstructorInitializerType == ConstructorInitializerType.This) {
        WriteKeyword(ConstructorInitializer.ThisKeywordRole);
    } else {
        WriteKeyword(ConstructorInitializer.BaseKeywordRole);
    }
    //.....
}

You write this and base as keywords, rather than as tokens. If we change that to WriteToken, they will become clickable and navigation will be hugely improved.

Could you please clarify whether that is an appropriate way to fix the issue and improve the navigation? If yes, I could create a PR. If not, please point me to the right direction.

P.S. I've also found that issue could be fixed in the TextTokenWriter.cs file in the ILSpy project. We could check if this is a special token and fix handle it as identifier. Is that more appropriate way?

Thanks!

zvirja commented 7 years ago

I've found that this is a ILSpy issue and fired appropriate PR there. Let me know if you also thing so.

dgrunwald commented 7 years ago

I think the fix in ILSpy is the correct solution.