Skip to content

Commit 672b20d

Browse files
committed
Clear problems view when a database is removed
This commit fixes the problem whereby a database is removed and the problems associated with queries run from that database stick around in the problems view. Also, once problems are cleared, we need to make sure that we uncheck the checkbox in the results view. This commit has several limitations: 1. There is duplicated code for message handling in both results.tsx and result-tables.tsx. 2. Problems are cleared whenever there is *any* change to any database. Ideally we should only clear problems when a database is removed and only problems associated with that database. I'll fix part of this in a future commit. Resolves #525
1 parent c83d1b3 commit 672b20d

4 files changed

Lines changed: 102 additions & 22 deletions

File tree

extensions/ql-vscode/src/interface-types.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,16 @@ export interface NavigatePathMsg {
127127
direction: number;
128128
}
129129

130+
export interface UntoggleShowProblemsMsg {
131+
t: 'untoggleShowProblems';
132+
}
133+
130134
export type IntoResultsViewMsg =
131135
| ResultsUpdatingMsg
132136
| SetStateMsg
133137
| ShowInterpretedPageMsg
134-
| NavigatePathMsg;
138+
| NavigatePathMsg
139+
| UntoggleShowProblemsMsg;
135140

136141
export type FromResultsViewMsg =
137142
| ViewSourceFileMsg

extensions/ql-vscode/src/interface.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,17 @@ export class InterfaceManager extends DisposableObject {
132132
this.navigatePathStep.bind(this, -1)
133133
)
134134
);
135+
136+
this.push(
137+
this.databaseManager.onDidChangeDatabaseItem(() => {
138+
// Consider only clearing database items when a database
139+
// is removed and only clearing items from that database.
140+
this._diagnosticCollection.clear();
141+
this.postMessage({
142+
t: 'untoggleShowProblems'
143+
});
144+
})
145+
);
135146
}
136147

137148
navigatePathStep(direction: number): void {

extensions/ql-vscode/src/view/result-tables.tsx

Lines changed: 80 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import {
1111
ALERTS_TABLE_NAME,
1212
SELECT_TABLE_NAME,
1313
getDefaultResultSetName,
14-
ParsedResultSets
14+
ParsedResultSets,
15+
IntoResultsViewMsg
1516
} from '../interface-types';
1617
import { PathTable } from './alert-table';
1718
import { RawTable } from './raw-results-table';
@@ -41,6 +42,7 @@ export interface ResultTablesProps {
4142
interface ResultTablesState {
4243
selectedTable: string; // name of selected result set
4344
selectedPage: string; // stringified selected page
45+
problemsViewSelected: boolean;
4446
}
4547

4648
const UPDATING_RESULTS_TEXT_CLASS_NAME = 'vscode-codeql__result-tables-updating-text';
@@ -101,7 +103,17 @@ export class ResultTables
101103
super(props);
102104
const selectedTable = props.parsedResultSets.selectedTable || getDefaultResultSet(this.getResultSets());
103105
const selectedPage = (props.parsedResultSets.pageNumber + 1) + '';
104-
this.state = { selectedTable, selectedPage };
106+
this.state = {
107+
selectedTable,
108+
selectedPage,
109+
problemsViewSelected: false
110+
};
111+
}
112+
113+
untoggleProbemsView() {
114+
this.setState({
115+
problemsViewSelected: false
116+
});
105117
}
106118

107119
private onTableSelectionChange = (event: React.ChangeEvent<HTMLSelectElement>): void => {
@@ -115,26 +127,39 @@ export class ResultTables
115127

116128
private alertTableExtras(): JSX.Element | undefined {
117129
const { database, resultsPath, metadata, origResultsPaths } = this.props;
130+
const handleCheckboxChanged = (e: React.ChangeEvent<HTMLInputElement>) => {
131+
if (e.target.checked === this.state.problemsViewSelected) {
132+
// no change
133+
return;
134+
}
135+
this.setState({
136+
problemsViewSelected: e.target.checked
137+
});
138+
if (resultsPath !== undefined) {
139+
vscode.postMessage({
140+
t: 'toggleDiagnostics',
141+
origResultsPaths: origResultsPaths,
142+
databaseUri: database.databaseUri,
143+
visible: e.target.checked,
144+
metadata: metadata
145+
});
146+
}
147+
};
118148

119-
const displayProblemsAsAlertsToggle =
120-
<div className={toggleDiagnosticsClassName}>
121-
<input type="checkbox" id="toggle-diagnostics" name="toggle-diagnostics" onChange={(e) => {
122-
if (resultsPath !== undefined) {
123-
vscode.postMessage({
124-
t: 'toggleDiagnostics',
125-
origResultsPaths: origResultsPaths,
126-
databaseUri: database.databaseUri,
127-
visible: e.target.checked,
128-
metadata: metadata
129-
});
130-
}
131-
}} />
132-
<label htmlFor="toggle-diagnostics">Show results in Problems view</label>
133-
</div>;
134-
135-
return <div className={alertExtrasClassName}>
136-
{displayProblemsAsAlertsToggle}
137-
</div>;
149+
return (
150+
<div className={alertExtrasClassName}>
151+
<div className={toggleDiagnosticsClassName}>
152+
<input
153+
type="checkbox"
154+
id="toggle-diagnostics"
155+
name="toggle-diagnostics"
156+
onChange={handleCheckboxChanged}
157+
checked={this.state.problemsViewSelected}
158+
/>
159+
<label htmlFor="toggle-diagnostics">Show results in Problems view</label>
160+
</div>
161+
</div>
162+
);
138163
}
139164

140165
getOffset(): number {
@@ -244,6 +269,40 @@ export class ResultTables
244269
}
245270
</div>;
246271
}
272+
273+
handleMessage(msg: IntoResultsViewMsg): void {
274+
switch (msg.t) {
275+
case 'untoggleShowProblems':
276+
this.setState({
277+
problemsViewSelected: false
278+
});
279+
break;
280+
281+
default:
282+
// noop
283+
}
284+
}
285+
286+
// TODO: Duplicated from results.tsx consider a way to
287+
// avoid this duplication
288+
componentDidMount(): void {
289+
this.vscodeMessageHandler = (evt) =>
290+
evt.origin === window.origin
291+
? this.handleMessage(evt.data as IntoResultsViewMsg)
292+
: console.error(`Invalid event origin ${evt.origin}`);
293+
294+
window.addEventListener('message', this.vscodeMessageHandler);
295+
}
296+
297+
componentWillUnmount(): void {
298+
if (this.vscodeMessageHandler) {
299+
window.removeEventListener('message', this.vscodeMessageHandler);
300+
}
301+
}
302+
303+
private vscodeMessageHandler:
304+
| ((ev: MessageEvent) => void)
305+
| undefined = undefined;
247306
}
248307

249308
class ResultTable extends React.Component<ResultTableProps, {}> {

extensions/ql-vscode/src/view/results.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ class App extends React.Component<{}, ResultsViewState> {
137137
case 'navigatePath':
138138
onNavigation.fire(msg);
139139
break;
140+
141+
case 'untoggleShowProblems':
142+
// noop
143+
break;
144+
140145
default:
141146
assertNever(msg);
142147
}

0 commit comments

Comments
 (0)