Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes LSP initialization issues and updates the project structure. The main purpose is to resolve GitHub issues #1 and #2, specifically using sage -python to start the LSP server and fixing venv creation logic.
Key changes:
- Modified LSP venv creation to use
sage -pythonfrom the conda environment instead of system Python - Fixed LSP startup flow to handle venv creation failures properly
- Reorganized documentation files from
readmes/todocs/directory
Reviewed Changes
Copilot reviewed 8 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/requirements.txt | Changed pygls version constraint from >=1.3.1 to exact version ==1.3.1 |
| src/extension.ts | Refactored LSP initialization logic: moved functions after command setup, added conda environment path requirement for venv creation, fixed cleanup using rmSync, improved error handling |
| package-lock.json | Updated lockfile format from v3 to v1, bumped version to 1.1.1, updated dependencies (@types/node, @types/vscode, undici-types, semver) |
| docs/SemanticHighlighting-zh-CN.md | Added Chinese documentation for known semantic highlighting bugs |
| docs/SemanticHighlighting-en.md | Added English documentation for known semantic highlighting bugs |
| docs/README-zh-CN.md | Added Chinese README with project overview and installation instructions |
| docs/CONTRIBUTING.md | Added build instructions for contributors |
| README.md | Updated documentation paths from readmes/ to docs/ |
| CHANGELOG.md | Added version 1.1.2 changelog entry documenting fixes and changes |
| .vscode/tasks.json | Added VSCode tasks configuration for development |
| .vscode/settings.json | Added VSCode workspace settings |
| .vscode/launch.json | Added VSCode debug launch configuration |
| .gitignore | Removed .vscode from gitignore to include development configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function createEnvLSP(envLSPpath: string, requirementsPath: string): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| const pythonCommand = process.platform === 'win32' ? 'python' : 'python3'; | ||
| const condaEnvPath = vscode.workspace.getConfiguration('sagemath-for-vscode.sage').get<string>('condaEnvPath'); | ||
| while (!condaEnvPath) { | ||
| vscode.commands.executeCommand('sagemath-for-vscode.selectCondaEnv'); | ||
| } | ||
| console.log(`Using Conda Env Path: ${condaEnvPath}`); | ||
|
|
There was a problem hiding this comment.
This while loop creates an infinite blocking loop. executeCommand is asynchronous and doesn't wait for user input. The loop will spin indefinitely without allowing the user to select an environment. Consider using an await with proper async handling or prompting the user and rejecting the promise if no environment is selected.
| function createEnvLSP(envLSPpath: string, requirementsPath: string): Promise<void> { | |
| return new Promise((resolve, reject) => { | |
| const pythonCommand = process.platform === 'win32' ? 'python' : 'python3'; | |
| const condaEnvPath = vscode.workspace.getConfiguration('sagemath-for-vscode.sage').get<string>('condaEnvPath'); | |
| while (!condaEnvPath) { | |
| vscode.commands.executeCommand('sagemath-for-vscode.selectCondaEnv'); | |
| } | |
| console.log(`Using Conda Env Path: ${condaEnvPath}`); | |
| async function createEnvLSP(envLSPpath: string, requirementsPath: string): Promise<void> { | |
| const pythonCommand = process.platform === 'win32' ? 'python' : 'python3'; | |
| let condaEnvPath = vscode.workspace.getConfiguration('sagemath-for-vscode.sage').get<string>('condaEnvPath'); | |
| if (!condaEnvPath) { | |
| await vscode.commands.executeCommand('sagemath-for-vscode.selectCondaEnv'); | |
| condaEnvPath = vscode.workspace.getConfiguration('sagemath-for-vscode.sage').get<string>('condaEnvPath'); | |
| if (!condaEnvPath) { | |
| throw new Error('No Conda environment selected.'); | |
| } | |
| } | |
| console.log(`Using Conda Env Path: ${condaEnvPath}`); | |
| return new Promise((resolve, reject) => { |
| // If LSP enabled | ||
| const useLSP = vscode.workspace.getConfiguration('sagemath-for-vscode.LSP').get<boolean>('useSageMathLSP', true); | ||
| if (!useLSP) { | ||
| vscode.window.showInformationMessage('SageMath Language Server is disabled. Please enable it in settings and restart extnsion to use LSP features.'); |
There was a problem hiding this comment.
Corrected spelling of 'extnsion' to 'extension'.
| vscode.window.showInformationMessage('SageMath Language Server is disabled. Please enable it in settings and restart extnsion to use LSP features.'); | |
| vscode.window.showInformationMessage('SageMath Language Server is disabled. Please enable it in settings and restart extension to use LSP features.'); |
| await createEnvLSP(envLSPpath, requirementsPath); | ||
| vscode.window.showInformationMessage('✅ Python venv created.'); | ||
| } catch (err) { | ||
| vscode.window.showErrorMessage(`❌ Failed to creat Python venv: \n${err}`); |
There was a problem hiding this comment.
Corrected spelling of 'creat' to 'create'.
| vscode.window.showErrorMessage(`❌ Failed to creat Python venv: \n${err}`); | |
| vscode.window.showErrorMessage(`❌ Failed to create Python venv: \n${err}`); |
| } | ||
|
|
||
| const envs: { name: string; path: string }[] = [{ "name": "Global Environment", "path": "" }]; | ||
| const globalEnv = { "name": "Global Environment", "path": "" }; |
There was a problem hiding this comment.
The globalEnv variable is defined but never used. It appears to have been removed from the environments array but the variable declaration remains. Either remove this unused variable or add it back to the envs array if it was removed by mistake.
| const globalEnv = { "name": "Global Environment", "path": "" }; |
| } | ||
| console.log(`Using Conda Env Path: ${condaEnvPath}`); | ||
|
|
||
| exec(`${condaEnvPath}/bin/sage -python -m venv ${envLSPpath}`, (error, stdout, stderr) => { |
There was a problem hiding this comment.
The condaEnvPath and envLSPpath variables are used directly in a shell command without proper escaping or validation. If these paths contain spaces or special shell characters, the command could fail or behave unexpectedly. Consider using proper shell escaping or passing arguments in array form if the exec API supports it.
[1.1.2] - 2025-11-05
Fixed
Changed
Added