Reduce number of `parse` calls by Discookie · Pull Request #58 · Ericsson/CodecheckerVSCodePlugin

Discookie · 2022-01-26T21:22:35Z

Fixes#39.

The extension no longer executes a parse call when:

Open files did not change
There are no files open
A changed window has 'output://' scheme - ref: Output window hides reports in current file #13

csordasmarton · 2022-01-28T08:11:27Z

+ const dummyData: ParseResult = { reports: [], version: 1 };
+ this.parseDiagnosticsData(JSON.stringify(dummyData));


I don't really like this approach that we are creating a dummy data only to call this function. Why do we need to call parseDiagnosticsData here? To initialize this._diagnosticEntries and other variables? In this case we can initialize these fields here too. Also if it contains some extra logic we can create a separate function for it and we can call it here and in the parseDiagnosticsData function.

Moved the field initializations here.
The parseMetadata does a little bit of extra processing when deciding whether this._selectedEntry should be kept, but selectedEntry cannot exist inside the unloading if, so it's no concern here.

Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

csordasmarton

LGTM!

Discookie · 2022-01-26T21:22:35Z

Fixes#39.

The extension no longer executes a parse call when:

Open files did not change
There are no files open
A changed window has 'output://' scheme - ref: Output window hides reports in current file #13

csordasmarton · 2022-01-28T08:11:27Z

+ const dummyData: ParseResult = { reports: [], version: 1 };
+ this.parseDiagnosticsData(JSON.stringify(dummyData));


I don't really like this approach that we are creating a dummy data only to call this function. Why do we need to call parseDiagnosticsData here? To initialize this._diagnosticEntries and other variables? In this case we can initialize these fields here too. Also if it contains some extra logic we can create a separate function for it and we can call it here and in the parseDiagnosticsData function.

Moved the field initializations here.
The parseMetadata does a little bit of extra processing when deciding whether this._selectedEntry should be kept, but selectedEntry cannot exist inside the unloading if, so it's no concern here.

Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

csordasmarton

LGTM!

Discookie · 2022-01-26T21:22:35Z

Fixes#39.

The extension no longer executes a parse call when:

Open files did not change
There are no files open
A changed window has 'output://' scheme - ref: Output window hides reports in current file #13

csordasmarton · 2022-01-28T08:11:27Z

+ const dummyData: ParseResult = { reports: [], version: 1 };
+ this.parseDiagnosticsData(JSON.stringify(dummyData));


I don't really like this approach that we are creating a dummy data only to call this function. Why do we need to call parseDiagnosticsData here? To initialize this._diagnosticEntries and other variables? In this case we can initialize these fields here too. Also if it contains some extra logic we can create a separate function for it and we can call it here and in the parseDiagnosticsData function.

Moved the field initializations here.
The parseMetadata does a little bit of extra processing when deciding whether this._selectedEntry should be kept, but selectedEntry cannot exist inside the unloading if, so it's no concern here.

Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

csordasmarton

LGTM!

Discookie · 2022-01-26T21:22:35Z

Fixes#39.

The extension no longer executes a parse call when:

Open files did not change
There are no files open
A changed window has 'output://' scheme - ref: Output window hides reports in current file #13

csordasmarton · 2022-01-28T08:11:27Z

+ const dummyData: ParseResult = { reports: [], version: 1 };
+ this.parseDiagnosticsData(JSON.stringify(dummyData));


I don't really like this approach that we are creating a dummy data only to call this function. Why do we need to call parseDiagnosticsData here? To initialize this._diagnosticEntries and other variables? In this case we can initialize these fields here too. Also if it contains some extra logic we can create a separate function for it and we can call it here and in the parseDiagnosticsData function.

Moved the field initializations here.
The parseMetadata does a little bit of extra processing when deciding whether this._selectedEntry should be kept, but selectedEntry cannot exist inside the unloading if, so it's no concern here.

Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

csordasmarton

LGTM!

Discookie · 2022-01-26T21:22:35Z

Fixes#39.

The extension no longer executes a parse call when:

Open files did not change
There are no files open
A changed window has 'output://' scheme - ref: Output window hides reports in current file #13

csordasmarton · 2022-01-28T08:11:27Z

+ const dummyData: ParseResult = { reports: [], version: 1 };
+ this.parseDiagnosticsData(JSON.stringify(dummyData));


I don't really like this approach that we are creating a dummy data only to call this function. Why do we need to call parseDiagnosticsData here? To initialize this._diagnosticEntries and other variables? In this case we can initialize these fields here too. Also if it contains some extra logic we can create a separate function for it and we can call it here and in the parseDiagnosticsData function.

Moved the field initializations here.
The parseMetadata does a little bit of extra processing when deciding whether this._selectedEntry should be kept, but selectedEntry cannot exist inside the unloading if, so it's no concern here.

Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

csordasmarton

LGTM!

Discookie · 2022-01-26T21:22:35Z

Fixes#39.

The extension no longer executes a parse call when:

Open files did not change
There are no files open
A changed window has 'output://' scheme - ref: Output window hides reports in current file #13

csordasmarton · 2022-01-28T08:11:27Z

+ const dummyData: ParseResult = { reports: [], version: 1 };
+ this.parseDiagnosticsData(JSON.stringify(dummyData));


I don't really like this approach that we are creating a dummy data only to call this function. Why do we need to call parseDiagnosticsData here? To initialize this._diagnosticEntries and other variables? In this case we can initialize these fields here too. Also if it contains some extra logic we can create a separate function for it and we can call it here and in the parseDiagnosticsData function.

Moved the field initializations here.
The parseMetadata does a little bit of extra processing when deciding whether this._selectedEntry should be kept, but selectedEntry cannot exist inside the unloading if, so it's no concern here.

Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

csordasmarton

LGTM!

Discookie · 2022-01-26T21:22:35Z

Fixes#39.

The extension no longer executes a parse call when:

Open files did not change
There are no files open
A changed window has 'output://' scheme - ref: Output window hides reports in current file #13

csordasmarton · 2022-01-28T08:11:27Z

+ const dummyData: ParseResult = { reports: [], version: 1 };
+ this.parseDiagnosticsData(JSON.stringify(dummyData));


I don't really like this approach that we are creating a dummy data only to call this function. Why do we need to call parseDiagnosticsData here? To initialize this._diagnosticEntries and other variables? In this case we can initialize these fields here too. Also if it contains some extra logic we can create a separate function for it and we can call it here and in the parseDiagnosticsData function.

Moved the field initializations here.
The parseMetadata does a little bit of extra processing when deciding whether this._selectedEntry should be kept, but selectedEntry cannot exist inside the unloading if, so it's no concern here.

Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

csordasmarton

LGTM!

Discookie · 2022-01-26T21:22:35Z

Fixes#39.

The extension no longer executes a parse call when:

Open files did not change
There are no files open
A changed window has 'output://' scheme - ref: Output window hides reports in current file #13

csordasmarton · 2022-01-28T08:11:27Z

+ const dummyData: ParseResult = { reports: [], version: 1 };
+ this.parseDiagnosticsData(JSON.stringify(dummyData));


I don't really like this approach that we are creating a dummy data only to call this function. Why do we need to call parseDiagnosticsData here? To initialize this._diagnosticEntries and other variables? In this case we can initialize these fields here too. Also if it contains some extra logic we can create a separate function for it and we can call it here and in the parseDiagnosticsData function.

Moved the field initializations here.
The parseMetadata does a little bit of extra processing when deciding whether this._selectedEntry should be kept, but selectedEntry cannot exist inside the unloading if, so it's no concern here.

Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

csordasmarton

LGTM!

Discookie added the bugfix label Jan 26, 2022

Discookie requested a review from csordasmarton January 26, 2022 21:22

csordasmarton suggested changes Jan 28, 2022
View reviewed changes

csordasmarton added this to the 1.0.1 milestone Jan 28, 2022

Discookie force-pushed the ericsson/fix-double-parse branch from 5b250e8 to c95cb6dCompare January 28, 2022 13:06

Discookie requested a review from csordasmarton January 28, 2022 13:10

Reduce number of parse calls
2421b52
Does not call parse: * When open files did not change * When there's no files open * When an opened window is an 'output://' window

Discookie force-pushed the ericsson/fix-double-parse branch from c95cb6d to 2421b52Compare January 28, 2022 13:11

csordasmarton approved these changes Jan 31, 2022
View reviewed changes

csordasmarton merged commit 65747d4 into Ericsson:mainJan 31, 2022