Is the expectation that other derives outside of Roslyn will use this? My worry is whether we're best off making these private and expect the derived type to re-query if it needs them; otherwise we're exposing more here than we may want.

My expectation would have been that all of this still lives in the base. The only bit that would be in the derived methods would be the stuff that starts on if (w.CurrentSolution.ContainsDocument(…)) stuff, where the input to the function is a DocumentId and the SourceTextContainer.

This is of course tricky because of the locking semantics, so you might have to have two methods that are implemented by derivers

void ApplyWorkspaceChangeUnderLock(Action<Workspace> w);

which just forwards to the VS workspace one that acquires the lock, and then:

void ApplyDocumentOpen(DocumentId, SourceTextContainer)

(referring to the earlier comment) -- start what is the derived behavior here -- keep the rest still in the base.

The reason being that sometimes we use "VisualStudioBlah" to generally mean an implementation that's used for all workspace implementations running in VS; this isn't quite the same.

Why an interface over just events? It seems neither client needed to use all of the methods, and this pattern doesn't scale well if we ever need to add another one. That the RDT events themselves are done via interfaces is just because events hadn't been invented back then.*

(* Well, yes, COM can do events too, but let's not talk about such things in polite company.)

I see now @tmat wanted it this way, and his argument is persuasive given what he commented on. I suspect part of the problem though was there were more events than necessary, and I guess I'm not a particularly strong believer in EventArgs being a useful pattern, so maybe that would have helped. :-)

My experience with C# eventing is a little limited - what is the alternative to event args?

You can be lazy and just not do the pattern. Just have delegate types that have the parameters you want.

I must admit: I'm curious what this would look like if we replaced this dictionary operating on cookies with file names. Would this then let us toss all the cookie passing in common helper? That'd be great then when we replace this with an editor API that wouldn't be talking about cookies anyways.

And maybe this isn't a strict requirement now, but if you squint a bit you could imagine this isn't a RunningDocumentTableEventsTracker, but just an generic abstraction over "give me the open files and let me know when that changes" -- we might be able to get rid of cookies entirely. That might be handy if we want to use the misc files workspace somewhere else since once this isn't touching the RDT directly it's pretty light on other VS tie-ins.

Feels a bit strange we explicitly cast this to IVsRunningDocumentTable4 -- let the helper just take the base one and it can cast it to whatever versions it needs.

Nitpick: use named arguments for passing the pszMkDocumentOld/New, so there's no chance these are flipped.

From the API perspective should this be called IsFileOpen?

Not just JIT time in this method -- avoids an entire assembly load.

Any resaon this isn't return an IEnumerable? IEnumerable of tuple here seems most appropriate.

(also means the enumeration is abortable...)

Or why can't a caller just enumerate just the file names, and they can call TryGetBufferFromMoniker themselves?

The reason I did it this way was to avoid having to return all three pieces of data. But an IEnumerable of tuple seems like a reasonable way to do it. Will update.

Couldn't a caller just do this via the APIs you already have?

Not sure comment makes sense here: might be better to say "when a file is renamed, the old document is removed and new document added by the workspace". QueueCheckForFilesBeingOpen isn't really "handing" the rename per se, it's just the helper.