From d0982f34a41280bf271f4b5a022e8a14eb33145c Mon Sep 17 00:00:00 2001 From: Jason Reed Date: Wed, 12 Aug 2020 11:51:23 -0400 Subject: [PATCH 1/3] Defunctionalize updating sort state This leads to less sharing of codepaths which is a little bad (slightly more repetition and rendundancy) but a lot good (can independently fix the way raw results are redisplayed so as to be actually correct). --- extensions/ql-vscode/src/interface.ts | 40 +++++++++++++++-------- extensions/ql-vscode/src/query-results.ts | 2 +- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/extensions/ql-vscode/src/interface.ts b/extensions/ql-vscode/src/interface.ts index 3d818c4e9..ccddebb45 100644 --- a/extensions/ql-vscode/src/interface.ts +++ b/extensions/ql-vscode/src/interface.ts @@ -29,6 +29,7 @@ import { RAW_RESULTS_PAGE_SIZE, INTERPRETED_RESULTS_PAGE_SIZE, ALERTS_TABLE_NAME, + RawResultsSortState, } from './interface-types'; import { Logger } from './logging'; import * as messages from './messages'; @@ -190,8 +191,8 @@ export class InterfaceManager extends DisposableObject { return this._panel; } - private async changeSortState( - update: (query: CompletedQuery) => Promise + private async changeInterpretedSortState( + sortState: InterpretedResultsSortState | undefined ): Promise { if (this._displayedQuery === undefined) { showAndLogErrorMessage( @@ -201,7 +202,28 @@ export class InterfaceManager extends DisposableObject { } // Notify the webview that it should expect new results. await this.postMessage({ t: 'resultsUpdating' }); - await update(this._displayedQuery); + this._displayedQuery.updateInterpretedSortState(sortState); + await this.showResults(this._displayedQuery, WebviewReveal.NotForced, true); + } + + private async changeRawSortState( + server: cli.CodeQLCliServer, + resultSetName: string, + sortState: RawResultsSortState | undefined + ): Promise { + if (this._displayedQuery === undefined) { + showAndLogErrorMessage( + 'Failed to sort results since evaluation info was unknown.' + ); + return; + } + // Notify the webview that it should expect new results. + await this.postMessage({ t: 'resultsUpdating' }); + this._displayedQuery.updateSortState( + server, + resultSetName, + sortState + ); await this.showResults(this._displayedQuery, WebviewReveal.NotForced, true); } @@ -235,18 +257,10 @@ export class InterfaceManager extends DisposableObject { this._panelLoadedCallBacks = []; break; case 'changeSort': - await this.changeSortState(query => - query.updateSortState( - this.cliServer, - msg.resultSetName, - msg.sortState - ) - ); + await this.changeRawSortState(this.cliServer, msg.resultSetName, msg.sortState); break; case 'changeInterpretedSort': - await this.changeSortState(query => - query.updateInterpretedSortState(this.cliServer, msg.sortState) - ); + await this.changeInterpretedSortState(msg.sortState); break; case 'changePage': if (msg.selectedTable === ALERTS_TABLE_NAME) { diff --git a/extensions/ql-vscode/src/query-results.ts b/extensions/ql-vscode/src/query-results.ts index f26a0a4a9..34fc27aa0 100644 --- a/extensions/ql-vscode/src/query-results.ts +++ b/extensions/ql-vscode/src/query-results.ts @@ -117,7 +117,7 @@ export class CompletedQuery implements QueryWithResults { this.sortedResultsInfo.set(resultSetName, sortedResultSetInfo); } - async updateInterpretedSortState(_server: cli.CodeQLCliServer, sortState: InterpretedResultsSortState | undefined): Promise { + async updateInterpretedSortState(sortState: InterpretedResultsSortState | undefined): Promise { this.interpretedResultsSortState = sortState; } } From 039343efa2515c6f9a47eac12e0ccb4c4cd50225 Mon Sep 17 00:00:00 2001 From: Jason Reed Date: Wed, 12 Aug 2020 12:08:32 -0400 Subject: [PATCH 2/3] Fix #527. --- extensions/ql-vscode/src/interface.ts | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/extensions/ql-vscode/src/interface.ts b/extensions/ql-vscode/src/interface.ts index ccddebb45..45239365d 100644 --- a/extensions/ql-vscode/src/interface.ts +++ b/extensions/ql-vscode/src/interface.ts @@ -219,12 +219,16 @@ export class InterfaceManager extends DisposableObject { } // Notify the webview that it should expect new results. await this.postMessage({ t: 'resultsUpdating' }); - this._displayedQuery.updateSortState( + await this._displayedQuery.updateSortState( server, resultSetName, sortState ); - await this.showResults(this._displayedQuery, WebviewReveal.NotForced, true); + // Sorting resets to first page, as there is arguably no particular + // correlation between the results on the nth page that the user + // was previously viewing and the contents of the nth page in a + // new sorted order. + await this.showPageOfRawResults(resultSetName, 0, true); } private async handleMsgFromView(msg: FromResultsViewMsg): Promise { @@ -437,7 +441,8 @@ export class InterfaceManager extends DisposableObject { */ public async showPageOfRawResults( selectedTable: string, - pageNumber: number + pageNumber: number, + sorted = false ): Promise { const results = this._displayedQuery; if (results === undefined) { @@ -459,8 +464,21 @@ export class InterfaceManager extends DisposableObject { if (schema === undefined) throw new Error(`Query result set '${selectedTable}' not found.`); + const getResultsPath = () => { + if (sorted) { + const resultsPath = results.sortedResultsInfo.get(selectedTable)?.resultsPath; + if (resultsPath === undefined) { + throw new Error(`Can't find sorted results for table ${selectedTable}`); + } + return resultsPath; + } + else { + return results.query.resultsPaths.resultsPath; + } + }; + const chunk = await this.cliServer.bqrsDecode( - results.query.resultsPaths.resultsPath, + getResultsPath(), schema.name, { offset: schema.pagination?.offsets[pageNumber], From 8c55e3ef2d54171e87dca2461e82fcc812f06b78 Mon Sep 17 00:00:00 2001 From: Jason Reed Date: Wed, 12 Aug 2020 12:25:20 -0400 Subject: [PATCH 3/3] Simplify argument passing --- extensions/ql-vscode/src/interface.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/src/interface.ts b/extensions/ql-vscode/src/interface.ts index 45239365d..22b645f36 100644 --- a/extensions/ql-vscode/src/interface.ts +++ b/extensions/ql-vscode/src/interface.ts @@ -207,7 +207,6 @@ export class InterfaceManager extends DisposableObject { } private async changeRawSortState( - server: cli.CodeQLCliServer, resultSetName: string, sortState: RawResultsSortState | undefined ): Promise { @@ -220,7 +219,7 @@ export class InterfaceManager extends DisposableObject { // Notify the webview that it should expect new results. await this.postMessage({ t: 'resultsUpdating' }); await this._displayedQuery.updateSortState( - server, + this.cliServer, resultSetName, sortState ); @@ -261,7 +260,7 @@ export class InterfaceManager extends DisposableObject { this._panelLoadedCallBacks = []; break; case 'changeSort': - await this.changeRawSortState(this.cliServer, msg.resultSetName, msg.sortState); + await this.changeRawSortState(msg.resultSetName, msg.sortState); break; case 'changeInterpretedSort': await this.changeInterpretedSortState(msg.sortState);