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

Relace - experimentation #204

Closed
wants to merge 8 commits into from
Closed

Relace - experimentation #204

wants to merge 8 commits into from

Conversation

Himanshu-Singh-Chauhan
Copy link

@Himanshu-Singh-Chauhan Himanshu-Singh-Chauhan commented Nov 30, 2024

closing this pr, we now have relace, but this is a good pr to refer to for relace and diff stuff.

  • relace first iteration
  • fast apply open diff with new temp file

Important

Adds 'Fast Apply' feature using Relace service for quick code changes with UI and backend integration.

  • Behavior:
    • Adds applyWithRelace, applyWithRelaceHorizontal, applyWithRelaceVertical, acceptRelaceDiff, and rejectRelaceDiff to ToIdeFromWebviewProtocol in ideWebview.ts.
    • Implements handlers for these methods in VsCodeMessenger to manage code changes and diffs.
    • Introduces getFastApplyChangesWithRelace in relace.ts to fetch modified content from a server.
  • UI:
    • Adds "Fast Apply" buttons to CodeBlockToolBar in CodeBlockToolbar.tsx for horizontal and vertical application of changes.
    • Provides options to accept or reject changes directly from the toolbar.
  • Utilities:
    • Adds extractCodeFromMarkdown to integrationUtils.ts to extract code from markdown text.
    • Introduces RelaceDiffManager in relaceDiffManager.ts to manage diff states.
  • Misc:
    • Changes SERVER_URL in parameters.ts to use a local development URL.

This description was created by Ellipsis for f70ebd3. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to f8ae29a in 1 minute and 29 seconds

More details
  • Looked at 291 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. extensions/vscode/src/util/integrationUtils.ts:74
  • Draft comment:
    The extractCodeFromMarkdown function may not handle multiple code blocks or code blocks without language specification correctly. Consider enhancing the regex to handle these cases.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The function extractCodeFromMarkdown is used to extract code from markdown text. However, the regex used might not cover all edge cases, such as multiple code blocks or code blocks without language specification.

Workflow ID: wflow_zECIHbCaWkYqOSRO


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

export const SERVER_URL = "https://stingray-app-gb2an.ondigitalocean.app/pearai-server-api2"
// export const SERVER_URL = "http://localhost:8000";
// export const SERVER_URL = "https://stingray-app-gb2an.ondigitalocean.app/pearai-server-api2"
export const SERVER_URL = "http://localhost:8000";
Copy link

Choose a reason for hiding this comment

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

Consider using an environment variable for SERVER_URL to allow flexibility between development and production environments.

@@ -332,6 +333,74 @@ export class VsCodeMessenger {
verticalDiffManager.streamEdit(prompt, modelTitle);
});

this.onWebview("applyWithRelace", async (msg) => {
Copy link

Choose a reason for hiding this comment

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

In applyWithRelace, handle the case where getFastApplyChangesWithRelace returns the original content due to an error. Notify the user that no changes were made.

): Promise<string> {
try {
const baseHeaders = await getHeaders();
const auth: any = await vscode.commands.executeCommand("pearai.getPearAuth");
Copy link

Choose a reason for hiding this comment

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

Avoid using any type for auth. Define a specific type or interface for better type safety.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on 4aeb511 in 35 seconds

More details
  • Looked at 70 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_NtyzorWOJ1sWCZzF


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor');
relaceDiffManager.clearDiffState();

this.webviewProtocol.request("setRelaceDiffState", {diffVisible: false, originalFileUri: "aasdfasdf", diffFileUri: "asdfasdf"});
Copy link

Choose a reason for hiding this comment

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

Avoid using placeholder strings like 'aasdfasdf' and 'asdfasdf'. Use meaningful values for the originalFileUri and diffFileUri in the setRelaceDiffState request.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 417dd44 in 39 seconds

More details
  • Looked at 281 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. gui/src/components/markdown/CodeBlockToolbar.tsx:170
  • Draft comment:
    The acceptRelaceDiff and rejectRelaceDiff functions are called with undefined as arguments. Ensure these functions do not rely on parameters that are not being passed, as this could lead to runtime errors.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. core/protocol/ideWebview.ts:22
  • Draft comment:
    The acceptRelaceDiff and rejectRelaceDiff types still include originalFileUri and diffFileUri parameters, which are no longer used. Consider removing these parameters to align with the updated implementation.
  • Reason this comment was not posted:
    Comment was on unchanged code.
3. extensions/vscode/src/extension/VsCodeMessenger.ts:352
  • Draft comment:
    Consider showing a user-friendly error message instead of throwing an error when originalContent is not found. This will improve the user experience.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The applyWithRelace function in VsCodeMessenger.ts has a check for originalContent being empty, but it throws an error instead of showing a user-friendly message. This could be improved for better user experience.

Workflow ID: wflow_eHS2VBtenKRLA6X5


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on f70ebd3 in 47 seconds

More details
  • Looked at 210 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. extensions/vscode/src/extension/VsCodeMessenger.ts:727
  • Draft comment:
    Ensure to handle the case where verticalDiffManager.createVerticalPerLineDiffHandler returns null to avoid potential runtime errors.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. extensions/vscode/src/extension/VsCodeMessenger.ts:729
  • Draft comment:
    Ensure to handle the case where verticalDiffManager.createVerticalPerLineDiffHandler returns null to avoid potential runtime errors.
  • Reason this comment was not posted:
    Marked as duplicate.
3. extensions/vscode/src/extension/VsCodeMessenger.ts:681
  • Draft comment:
    Ensure to handle the case where verticalDiffManager.createVerticalPerLineDiffHandler returns null to avoid potential runtime errors.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_ciIBoxaYqbvVABau


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

yield { type: 'new', line: modDiff[diffJ++] };
} else if (diffJ >= modDiff.length) {
// Add CodeLens for deleted line
vscode.languages.registerCodeLensProvider({ scheme: 'file' }, {
Copy link

Choose a reason for hiding this comment

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

Registering a CodeLens provider inside the generator function for each deleted line is inefficient. Consider moving this logic outside the generator to avoid performance issues.

@Himanshu-Singh-Chauhan Himanshu-Singh-Chauhan changed the title him/relace Relace - experimentation Dec 6, 2024
@Himanshu-Singh-Chauhan Himanshu-Singh-Chauhan marked this pull request as draft December 6, 2024 05:36
@Himanshu-Singh-Chauhan
Copy link
Author

this is a good pr to refer to for relace and diff stuff.
closing this now, we now have relace.

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