Conversation
There was a problem hiding this comment.
Pull request overview
This PR releases version 1.1.3 of the SageMath for VSCode extension, which includes fixes for issue #4, auto-removal of intermediate .sage.py files after execution, and expanded semantic highlighting support.
- Auto-remove
.sage.pyfiles generated by SageMath after running code - Add RR (real numbers) and CC (complex numbers) class definitions to predefinition.py for semantic highlighting
- Update documentation links from relative to absolute GitHub URLs
- Change terminal focus behavior when running SageMath files
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension.ts | Adds auto-removal of .sage.py files via shell command and changes terminal.show() to preserve editor focus |
| src/server/predefinition.py | Adds RR and CC class definitions for semantic highlighting support |
| src/server/lsp.py | Updates LSP server version number from 1.1.1 to 1.1.3 |
| package.json | Updates extension version from 1.1.1 to 1.1.3 |
| README.md | Updates relative documentation links to absolute GitHub URLs and adds feature entry for auto-remove functionality |
| CHANGELOG.md | Adds changelog entry for version 1.1.3 with fixes and new features, and updates previous entries with PR references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const filePath = editor.document.uri.fsPath; | ||
| const dirpath = dirname(filePath); | ||
| const command = `cd '${dirpath}' && ${SageMathPath} '${filePath}'`; | ||
| const command = `cd '${dirpath}' && ${SageMathPath} '${filePath}'; rm -f '${filePath}.py'`; |
There was a problem hiding this comment.
The rm -f command used here is Linux/Unix-specific and will fail on Windows. According to the README, the platform badge indicates this extension is for Linux only. However, VS Code extensions typically run cross-platform unless explicitly limited. Consider using the Node.js fs.rmSync() API (which is already imported in line 3) to perform this operation in a cross-platform manner, or add explicit Windows support by using a cross-platform shell command approach.
| const filePath = editor.document.uri.fsPath; | ||
| const dirpath = dirname(filePath); | ||
| const command = `cd '${dirpath}' && ${SageMathPath} '${filePath}'`; | ||
| const command = `cd '${dirpath}' && ${SageMathPath} '${filePath}'; rm -f '${filePath}.py'`; |
There was a problem hiding this comment.
The .sage.py file will be removed even if the SageMath execution fails. In shell command chaining with semicolons, the second command always runs regardless of the first command's exit status. Consider using && instead of ; before the rm command to only remove the file if SageMath runs successfully. This prevents removing the intermediate file when it might be useful for debugging a failed run.
| ### Added | ||
|
|
||
| - Auto-remove `.sage.py` files after running SageMath | ||
| - Add something to `predefinition.py` |
There was a problem hiding this comment.
The changelog entry "Add something to predefinition.py" is vague and unprofessional. It should specifically describe what was added: "Add RR and CC class definitions to predefinition.py" to provide clear documentation of the change.
| - Add something to `predefinition.py` | |
| - Add RR and CC class definitions to `predefinition.py` |
|
|
||
| - Auto-remove `.sage.py` files after running SageMath | ||
| - Add something to `predefinition.py` | ||
|
|
There was a problem hiding this comment.
The change to terminal.show(true) (adding the true parameter to preserve focus on the editor) is not documented in the CHANGELOG.md. This is a user-facing behavioral change that should be documented, especially since it relates to fixing issue #4.
| ### Changed | |
| - Show the SageMath terminal without stealing focus from the editor (using `terminal.show(true)`) when running SageMath, related to [#4](https://github.com/SeanDictionary/SageMath-for-VScode/issues/4) |
[1.1.3] - 2025-12-27
Fixed
Added
.sage.pyfiles after running SageMathpredefinition.py