Move todo comments analyzer to Editor Features

Can this not be at the Features or even Workspace level? I don't see anything Editor specific about this (i.e. using buffers/views).

Move todo comments analyzer to Editor Features

Can this not be at the Features or even Workspace level? I don't see anything Editor specific about this (i.e. using buffers/views).

Updated. Cunningham's Law strikes again :)

@sharwell It's not clear to me why we need to do that before merging this PR, given that it wasn't a goal of this PR nor is made any more difficult by this PR.

/azp run

@davidwengier It's not clear what the purpose of this pull request is. AbstractTodoCommentsIncrementalAnalyzer already lives in the features layer, and the changes proposed by this pull request do not align with the claim that it removes the need to implement these types separately (as the types are not exposed). Even if IVTs are involved, it's not desirable to adjust the code in ways that specifically facilitate increased use of APIs through IVTs.

@davidwengier David Wengier FTE It's not clear what the purpose of this pull request is.

The purpose is solely to prevent VSMac having to source-copy the incremental analyzer and analyzer provider. It comes from a request here if you have access.

it's not desirable to adjust the code in ways that specifically facilitate increased use of APIs through IVTs.

I agree with the spirit of this comment, but when the alternative option is copying source code which then becomes stale, I think allowing the IVT is the objectively better choice. I think thats especially true for something like this which is a pure move. If I'd left the namespaces in tact in the files there would have been no code changes at all.

For good or bad VSMac has extensive use of IVTs and I don't feel that blocking individual PRs does anything other than make life harder for the VSMac team, or in some cases prevents them shipping software. There was a larger conversation around trying to move them to external access but it was ruled out because the scope is too large, and the resources too little.

In this case it seems desirable to keep the current form. When we do move the implementation, it should be moved and exposed only through an external access layer which VS Mac can use. Otherwise we're just going to have to do the same work twice and break VS Mac both times.

Desirable to who? The VS Mac team have asked for it (that comment I linked to is an open PR on their end, refactoring their todo comment implementation) and yes this will break VS Mac, but i'm the one who fixes those breaks, so if you're blocking my PR in order to save me work, you may rest assured that I considered that in advance 😛