Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer RichEditBrackets creation #38659

Merged
merged 3 commits into from Nov 27, 2017

Conversation

mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Nov 17, 2017

Bug
RichEditBrackets are currently created eagerly pretty early in startup

fix
Lazily create RichEditBrackets instead

@mjbvz mjbvz force-pushed the defer-RichEditBrackets-creation branch from d1e7f97 to a34efaf Compare November 17, 2017 22:30
@jrieken jrieken removed their request for review November 20, 2017 08:51
Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

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

👍 LGTM, thank you! Just a small preference of mine to initialize all fields in the ctor please.

public readonly electricChars: { [key: string]: boolean; };

private readonly _languageIdentifier: LanguageIdentifier;
private _electricChars: { [key: string]: boolean; };
Copy link
Member

Choose a reason for hiding this comment

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

Small nit-pick: To avoid a hidden class mutation at runtime, I prefer to have all members initialized in the ctor. Can you please add this._electricChars = null;

@@ -46,18 +46,20 @@ export interface IIndentConverter {
export class RichEditSupport {

private readonly _conf: LanguageConfiguration;
private readonly _languageIdentifier: LanguageIdentifier;
private _brackets: RichEditBrackets;
private _electricCharacter: BracketElectricCharacterSupport;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

Can you please add this._brackets = null; this._electricCharacter = null;

@alexdima alexdima self-assigned this Nov 24, 2017
**Bug**
`RichEditBrackets` are currently created eagerly pretty early in startup. This can be a fairly call as well.

**fix**
Lazily create `RichEditBrackets` instead
@mjbvz mjbvz force-pushed the defer-RichEditBrackets-creation branch from c4df90a to 812128c Compare November 27, 2017 18:32
@mjbvz mjbvz added this to the November 2017 milestone Nov 27, 2017
@mjbvz mjbvz merged commit 276f8df into microsoft:master Nov 27, 2017
@alexdima
Copy link
Member

Nice!

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants