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

Add GenAIScript task provider and CLI configuration in VSCode extension #709

Merged
merged 8 commits into from
Sep 13, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Sep 13, 2024

This pull request adds a new task provider and CLI configuration for the GenAIScript extension in VSCode. It includes the following changes:

  • Added a new task provider for GenAIScript.

  • Added CLI configuration options for the GenAIScript extension.

  • Refactored the task provider and nodehost code.

Previously, when marking/unmarking a folder as viewed, a request was sent for every single file. This PR ensures that only one request is sent when marking/unmarking a folder as viewed.

  • A utility function quoteify was extracted from nodehost.ts and moved to a utility module util.ts. This function provides smart quotations in strings for improved command handling. 🎯
  • The package.json for the vscode package has been extended with taskDefinitions that allows for running GenAIScript scripts as tasks in the environment. This will permit easier interaction with tasks directly from VSCode. 👩‍💻🚀
  • A new module config.ts was added to the vscode package. This module imports and uses various constants also used in the core. Primarily, it provides a function resolveCli for determining the correct cli path and version with additional logic to warn if the CLI version is outdated. This ensures up-to-date working environment. ⚙️
  • The extension.ts now includes the activateTaskProvider function from a new module taskprovider.ts. This function is part of the main extension activation routine in VSCode and marks an additional extension feature. 🛠
  • A new TaskProvider was created with methods to provide and resolve tasks in the taskprovider.ts module. This allows for direct execution of tasks within VSCode, bringing closer integration with the GenAIScript system. With this, you can directly map and run GenAIScripts tasks from the VSCode tasks interface. 🗂️🔀

The changes do not appear to be 'user-facing', as they relate to the internal workings rather than connection or template public APIs. They'll significantly improve the productivity of developers working with GenAIScripts in a VSCode environment by allowing tasks to be run directly within VSCode. 🌟

generated by pr-describe

Copy link

The changes in the GIT_DIFF include:

  1. The function quoteify has been moved from nodehost.ts to util.ts. This makes the function reusable across different modules.

  2. A new TypeScript file config.ts was added in the vscode package. The resolveCli function in this file fetches CLI path and version from the workspace configuration and checks if the CLI version satisfies a minimum version requirement.

  3. In extension.ts, a new task provider is activated by invoking activeTaskProvider(state).

  4. A new TypeScript file taskprovider.ts has been added. The activeTaskProvider function in this file registers a new task provider that provides tasks based on project templates.

Overall, the changes seem to be well-structured and purposeful, enhancing the functionality and reusability of the code.

However, I have some concerns:

  1. The quoteify function in the util.ts file is not protected against undefined or null inputs. It's possible that an error will be thrown if null or undefined is passed to this function.

  2. In the config.ts file, semverSatisfies is used without error handling. If the version string is not in the correct format, this can lead to an error.

  3. In taskprovider.ts, the relative path scriptp is calculated but not checked for validity. If the script filename is not within the project folder, this could lead to problems.

Here are some potential code improvement suggestions:

diff --git a/packages/core/src/util.ts b/packages/core/src/util.ts
index c076ddda..c076ddda 100644
--- a/packages/core/src/util.ts
+++ b/packages/core/src/util.ts
@@ -196,6 +196,10 @@ export function logError(msg: string | Error | SerializedError) {
+export function quoteify(a: string) {
+    if (!a) return;
+    return /\s/.test(a) ? `"${a}"` : a
+}

diff --git a/packages/vscode/src/config.ts b/packages/vscode/src/config.ts
index a0bfe169..a0bfe169 100644
--- a/packages/vscode/src/config.ts
+++ b/packages/vscode/src/config.ts
@@ -0,0 +1,22 @@
+ if (!semverSatisfies(cliVersion, ">=" + gv.major + "." + gv.minor)) {
+    try {
+        vscode.window.showWarningMessage(
+            TOOL_ID +
+                ` - genaiscript cli version (${cliVersion}) outdated, please update to ${CORE_VERSION}`
+        )
+    } catch (err) {
+        console.error('Invalid version format: ', err);
+    }
+}

diff --git a/packages/vscode/src/taskprovider.ts b/packages/vscode/src/taskprovider.ts
index 5d268208..5d268208 100644
--- a/packages/vscode/src/taskprovider.ts
+++ b/packages/vscode/src/taskprovider.ts
@@ -0,0 +1,52 @@
+ const scriptp = host.path.relative(
+     host.projectFolder(),
+     script.filename
+ )
+ if (scriptp.startsWith('..')) {
+     console.error('Invalid script filename: ', script.filename);
+     return;
+ }

Other than these points, LGTM 🚀.

generated by pr-review

code,
}
annotations.add(annotation)
}

Choose a reason for hiding this comment

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

There is no error handling for the case when the severity is not found in the sevMap. This could lead to undefined behavior if an unexpected severity value is encountered. Consider adding error handling or a default case. 😊

generated by pr-review-commit missing_error_handling

@@ -49,12 +69,11 @@ export function parseAnnotations(text: string): Diagnostic[] {
message,
code,
}
const key = JSON.stringify(annotation)
annotations[key] = annotation
annotations.add(annotation)
return ""
}
)

Choose a reason for hiding this comment

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

The code for parsing GitHub and Azure DevOps annotations is almost identical. Consider refactoring this into a separate function to avoid code duplication. This will make the code easier to maintain and less prone to errors. 😇

generated by pr-review-commit duplicate_code

focus: true,
showReuseMessage: false,
clear: true,
}

Choose a reason for hiding this comment

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

There is no error handling for the case when the host.path.relative function fails or returns an unexpected result. This could lead to unexpected behavior if the relative path cannot be correctly determined. Consider adding error handling or validation to ensure the relative path can be correctly determined. 😊

generated by pr-review-commit missing_error_handling

@pelikhan pelikhan merged commit b0a7e99 into main Sep 13, 2024
8 of 10 checks passed
@pelikhan pelikhan deleted the taskprovider branch September 13, 2024 18:04
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.

1 participant