Skip to content

Generalized Output APIs.#9060

Merged
kittaakos merged 1 commit intoeclipse-theia:masterfrom
kittaakos:relax-output-api
Feb 23, 2021
Merged

Generalized Output APIs.#9060
kittaakos merged 1 commit intoeclipse-theia:masterfrom
kittaakos:relax-output-api

Conversation

@kittaakos
Copy link
Contributor

So that extenders can subclass and change the scheme.

Signed-off-by: Akos Kitta kittaakos@typefox.io

What it does

Generalizes the Output APIs. The scheme should have a type of string instead of 'output'.

Basically, I want to do something like this:

class MyEditorModelFactory extends OutputEditorModelFactory { readonly scheme: 'my-schem'; }

How to test

The code should compile.

Review checklist

Reminder for reviewers

So that extenders can subclass and change the `scheme`.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM!

@vince-fugnitto vince-fugnitto added extensibility issues to simplify ability to extend Theia output issues related to the output labels Feb 12, 2021
Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

LGTM, I am surprised as I would have assumed TS would under-specify automatically?

edit: TIL https://www.typescriptlang.org/docs/handbook/literal-types.html#literal-narrowing

@kittaakos kittaakos merged commit 7460e6f into eclipse-theia:master Feb 23, 2021
@kittaakos kittaakos deleted the relax-output-api branch February 23, 2021 13:01
@paul-marechal paul-marechal added this to the 1.11.0 milestone Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensibility issues to simplify ability to extend Theia output issues related to the output

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants