-
Notifications
You must be signed in to change notification settings - Fork 3
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
[workspace] show rich difference info on save #17
base: main
Are you sure you want to change the base?
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Run & review this pull request in StackBlitz Codeflow. |
Your Render PR Server URL is https://pyth-on-line-pr-17.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cqjr7fdds78s73eqdu60. |
Reviewer's Guide by SourceryThis pull request enhances the workspace save functionality by introducing a new File-Level Changes
Tips
|
✅ Deploy Preview for pyth-on-line ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @CNSeniorious000 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
@@ -3,5 +3,5 @@ import type { PyProxy } from "pyodide/ffi"; | |||
export class WorkspaceAPI extends PyProxy { | |||
close(): void; | |||
sync(sources: Record<string, string>, reload = true): void; | |||
save(path: string, content: string, reload = true): void; | |||
save(path: string, content: string, reload = true): string | void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Clarify the return type of save
method.
The save
method now returns string | void
. It would be helpful to document under what conditions each type is returned to avoid confusion.
save(path: string, content: string, reload = true): string | void; | |
/** | |
* Saves the content to the specified path. | |
* | |
* @param path - The path where the content should be saved. | |
* @param content - The content to save. | |
* @param reload - Whether to reload after saving. Defaults to true. | |
* @returns A string message if an error occurs, otherwise void. | |
*/ | |
save(path: string, content: string, reload = true): string | void; |
c60019a
to
d114c3d
Compare
b1c4df1
to
5e5ee4f
Compare
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @CNSeniorious000 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
Summary by Sourcery
Enhance the workspace save functionality to provide rich difference information when saving files. Introduce a new diff function to compute text differences and update the Workspace.svelte component to display these differences in toast notifications. Adjust styling in CodeBlock.svelte and md.css for better text presentation.
New Features:
Enhancements: