Conversation
| private readonly IAsynchronousOperationListener _asyncListener; | ||
| protected readonly IThreadingContext _threadingContext; | ||
| protected readonly IViewTagAggregatorFactoryService _tagAggregatorFactoryService; | ||
| protected readonly IAsynchronousOperationListener _asyncListener; |
There was a problem hiding this comment.
protected members should not use _ names. Use PascalCasing in this case.
| if (!textView.TextBuffer.GetFeatureOnOffOption(EditorComponentOnOffOptions.Adornment)) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
this is code you likely want to run in any subclass. so instead of this pattern, have a non virtual TextViewCreated that does this logic. Then have it call into a new protected abstract method that all subclasses have to implement that then doesn't need to duplciate this logic.
| internal abstract class AdornmentManager<T> where T : GraphicsTag | ||
| { | ||
| private readonly object _invalidatedSpansLock = new object(); | ||
| protected readonly object _invalidatedSpansLock = new object(); |
There was a problem hiding this comment.
very wary about exposing a lock.
There was a problem hiding this comment.
was not used by the subclasses, made it private readonly
|
|
||
| /// <summary>Notification system about operations we do</summary> | ||
| private readonly IAsynchronousOperationListener _asyncListener; | ||
| protected IAsynchronousOperationListener AsyncListener { get; } |
There was a problem hiding this comment.
are all of these needed by the subclasses?
There was a problem hiding this comment.
I made the items that are not used by the subclass private readonly again.
| : base(threadingContext, textView, tagAggregatorFactoryService, asyncListener, adornmentLayerName) | ||
| { | ||
| } | ||
| } |
There was a problem hiding this comment.
this doesn't appear to be using any of the protected members of the base class. indeed, this type doesn't seem to have any useful logic/state in it at all. i'm not sure why it needs to exist.
There was a problem hiding this comment.
Added the method that differentiates it from inline errors.
Hola!
I separated out the work that was done to make the adornment manager abstract in its own PR. This was done so it could be reused for the inline diagnostics feature.
The line separator feature needed some updates per this change.