We already have the structure internally, we just need to expose it.
NIT: the comment looks open to misinterpretation, "no structured link" refers to the fact the clients don't support it, but could be read that we don't know which notes correspond to a main diagnostic.
Maybe rephrase to something link "If the client does not support structured links …"?
NIT: maybe call OutFn and return here to avoid checking for EmitRelatedLocations again?
|310 ↗||(On Diff #193708)|
NIT: seems unrelated. Maybe revert?
Maybe omit the std::tie()-based comparison altogether?
maybe use testPath() here to avoid PP directives?
Maybe vlog? This is what we use for dropped diagnostics, should probably stick to the same level with dropped notes (even though the dropped notes probably come up less often in practice).
As with the other comment - you're right, and we'll drop these if we refactor - I don't think there's any need to drop them now, though.
It's more readable than full path - e.g. if the main TU is foo.cc and includes foo.h, this is "foo.h".
It's true that if we want to flatten as another pass, we're not going to make use of this information. I'd rather change that in the flatten-as-another-pass patch, so we can see whether the damage to output is worth the refactoring.
Discussed further offline - it's not clear that expressing the flattening as LSP diagnostic -> LSP diagnostic is better than the current Diag -> LSP diagnostic.
So that followup probably won't happen, and there isn't that much to be gained from "unifying" the behavior in !AbsFile or File!=AbsFile cases.