-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lsp code action #81
Lsp code action #81
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, ich befürchte ich habe das Feature falsch verstanden. Das sind Quick Fixes. Die sind auch gut, aber ich hatte eher an sowas gedacht wie Unit Tests direkt vom Code heraus zu starten. Das werden wohl eher die Code Lenses sein.
So wie es momentan implementiert ist, würde das bedeuten, dass wir eine feste Menge von Code Actions haben, die wir immer abfragen und denen die Ranges mitgeben.
/// Dictionary of executable actions. | ||
/// The key is the action identifier, and the value is the action executor. | ||
/// </summary> | ||
public virtual Dictionary<string, Func<object[], object>> ExecutableCodeActions { get; } = new Dictionary<string, Func<object[], object>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mit object[]
verlieren wir jegliche Typsicherheit! Außerdem kann jeder das Dictionary zu jedem Zeitpunkt löschen. Virtual macht hier glaube ich auch mehr Schaden als Nutzen. Daher würde ich gern
- Das Dictionary entweder mindestens mal protected machen
- Das virtual rausnehmen
- Eine passendere Signatur als
Func<object[],object>
- Alternativ könnte man auch nur Methoden protected anbieten, um Code Actions hinzuzufügen.
Die CodeActions hängen jetzt nur an den Regeln. Ich könnte auch noch, falls nötig, die CodeLenses einfügen, da die sehr ähnlich zu den CodeActions funktionieren. |
Code Lenses wären super :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sieht sehr gut aus, ich habe aber trotzdem noch ein paar Kommentare :)
/// <inheritdoc /> | ||
public override void AddCodeLenses(ICollection<CodeLensInfo> codeLenses) | ||
{ | ||
if (Inner != null && Inner.IsActive) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Der Check für IsActive ist wegen Lookaheads? Ich glaube das wäre einen Kommentar wert.
No description provided.