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

Disable the automodel button if there are no methods that can be modeled #3377

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

robertbrignull
Copy link
Contributor

Builds upon #3376

If there are no methods that can be modeled with AI, then disable the button. This hopefully avoids the situation where the user clicks the button but it appears to do nothing.

Luckily we can quite easily reuse the getCandidates method so we avoid having two implementations on whether there are methods that can be modeled.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@robertbrignull robertbrignull requested a review from a team as a code owner February 20, 2024 12:38
@@ -14,6 +14,7 @@ import {
} from "@vscode/webview-ui-toolkit/react";
import type { ModelEditorViewState } from "../../model-editor/shared/view-state";
import type { AccessPathSuggestionOptions } from "../../model-editor/suggestions";
import { getCandidates } from "../../model-editor/auto-model";
Copy link
Contributor Author

@robertbrignull robertbrignull Feb 20, 2024

Choose a reason for hiding this comment

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

I'm getting lots of errors around types when compiling the view and I wonder if this import is causing it. Presumably because we're importing code meant for NodeJS in a browser environment. The auto-model.ts file does import a bunch of other stuff too, even though getCandidates itself it pretty innocent.

../codeql-cli/cli.ts:10:8 - error TS1259: Module '"/Users/robertbrignull/github/vscode-codeql/extensions/ql-vscode/node_modules/tree-kill/index"' can only be default-imported using the 'allowSyntheticDefaultImports' flag

10 import tk from "tree-kill";
          ~~

  ../../node_modules/tree-kill/index.d.ts:13:1
    13 export = treeKill;
       ~~~~~~~~~~~~~~~~~~
    This module is declared with 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.

../databases/local-databases/database-manager.ts:2:8 - error TS1192: Module '"vscode"' has no default export.

2 import vscode from "vscode";
         ~~~~~~

../databases/local-databases/database-manager.ts:73:31 - error TS7006: Parameter 'e' implicitly has an 'any' type.

73     const disposable = event((e) => {
                                 ~

../databases/local-databases/database-manager.ts:507:65 - error TS7006: Parameter 'folder' implicitly has an 'any' type.

507     return (vscode.workspace.workspaceFolders || []).findIndex((folder) =>
                                                                    ~~~~~~

../databases/local-databases/database-manager.ts:576:8 - error TS7006: Parameter 'folder' implicitly has an 'any' type.

576       (folder) => item.belongsToSourceArchiveExplorerUri(folder.uri),
           ~~~~~~

../local-queries/results-view.ts:198:55 - error TS7031: Binding element 'kind' implicitly has an 'any' type.

198       this.databaseManager.onDidChangeDatabaseItem(({ kind }) => {
                                                          ~~~~

../packaging/qlpack-file-loader.ts:6:34 - error TS2732: Cannot find module './qlpack-file.schema.json'. Consider using '--resolveJsonModule' to import module with '.json' extension.

6 import qlpackFileSchemaJson from "./qlpack-file.schema.json";
                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 7 errors in 4 files.

Errors  Files
     1  ../codeql-cli/cli.ts:10
     4  ../databases/local-databases/database-manager.ts:2
     1  ../local-queries/results-view.ts:198
     1  ../packaging/qlpack-file-loader.ts:6

Base automatically changed from robertbrignull/dont-model-twice to main February 20, 2024 13:57
Copy link
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

LGTM

@robertbrignull robertbrignull merged commit fee7eae into main Feb 21, 2024
15 checks passed
@robertbrignull robertbrignull deleted the robertbrignull/disable-automodel-button branch February 21, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants