microsoft / genaiscript

Generative AI Scripting
https://microsoft.github.io/genaiscript/
MIT License
81 stars 22 forks source link

centralize vscode.Uri usage #539

Closed pelikhan closed 3 weeks ago

pelikhan commented 3 weeks ago

In summary, these changes seem to centralize the creation of URIs in the VSCodeHost class, making the code more maintainable and consistent. ๐ŸŽฏ

generated by pr-describe

pelikhan commented 3 weeks ago

/genai-describe

pelikhan commented 3 weeks ago

/genai-review

github-actions[bot] commented 3 weeks ago

The changes in the GIT_DIFF show that the developer is refactoring the code to use a new method toUri from the host object to create URIs instead of using vscode.Uri.file. This is a good change as it centralizes the URI creation logic in one place, making the code more maintainable.

However, there is a potential issue with the toUri method. It checks if the input string is an absolute path or a URL, but it does not handle relative paths. If a relative path is passed to this method, it will be treated as an absolute path, which could lead to incorrect URIs.

Here is a suggested fix:

toUri(filenameOrUrl: string): vscode.Uri {
    const folder = this.projectUri;
    if (!filenameOrUrl) return folder;
    if (/^[a-z][a-z0-9+\-.]*:\/\//.test(filenameOrUrl))
        return vscode.Uri.parse(filenameOrUrl, true);
    if (this.path.isAbsolute(filenameOrUrl))
        return vscode.Uri.file(filenameOrUrl);
    else return vscode.Uri.file(path.join(folder.fsPath, filenameOrUrl));
}

This fix uses the path.join method to correctly handle relative paths.

Other than this issue, the changes look good. The developer has also removed some commented-out code, which is a good practice as it keeps the codebase clean.

So, with the above fix, LGTM :rocket:

generated by pr-review