Skip to content

Use new document formatting service when generating types#55463

Merged
davidwengier merged 21 commits intodotnet:mainfrom
davidwengier:FormatNewDocumentFromGenerateType
Aug 10, 2021
Merged

Use new document formatting service when generating types#55463
davidwengier merged 21 commits intodotnet:mainfrom
davidwengier:FormatNewDocumentFromGenerateType

Conversation

@davidwengier
Copy link
Member

@davidwengier davidwengier commented Aug 6, 2021

This builds on #55432 so only review from 6342b87 (#55463) onwards. Will rebase etc. when that is merged.

This makes Generate Type use the new document formatting service, which realistically means Generate Type now supports file header templates from .editorconfig, and using directive placement options.

@davidwengier davidwengier requested a review from a team as a code owner August 6, 2021 13:03
@ghost ghost added the Area-IDE label Aug 6, 2021
{
}

protected override string Language => LanguageNames.CSharp;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting. do you need this given that the service was already exported for C# anyways?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, dumb question, but does your system make sure that if it can't get an INewDocumentFormattingService that that's ok? we need to make sure we don't break F#/TS

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need this

This is needed to filter the providers by metadata in the base class. If they were filtered at this level, it would have to be in the base(..) constructor call, which would defeat the purpose of the lazy (I think?)

var addedDocument = forkedSolution.GetDocument(documentId)!;

// Call out to various new document formatters to tweak what they want
var formattingService = addedDocument.GetRequiredLanguageService<INewDocumentFormattingService>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what ensures that this exists? for example, for F#/TS?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I wrong in ascribing meaning to the absence of F#/TS implementations of AbstractEditorFactory? I guess they could use IVT.. It didn't seem like much of the other code was defensive though. Surely document.GetRequiredSyntaxRoot would fail for TS documents?

I can just make it resilient anyway though, this is the only spot that would need to change anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants