Apply IntelliJ formatting to the edited code snippet that comes from the agent #1454

odisseus · 2024-05-06T16:38:11Z

Test plan

Tested manually, mostly on this file.

Things that are confirmed to work (prompts might need to be reworded)

Undoing these operations also works perfectly.

Select all text, prompt "Remove all blank lines".
- The blank lines will be removed only inside the methods. The blank lines between the methods are removed by the agent but restored by the formatter.
Select a whole method, prompt "Remove all blank lines".
Select a piece of the code that references a variable, prompt "Check that the is not null".
- This should add some lines of code, growing the selection accordingly.

Intentionally misformatting the file

This flow technically works as expected, but the result might confuse the user.

Open a Java file. Make sure that it has formatting problems, e.g. multiple statements (ending with ;) in the same line.
Select a few lines above and below the misformatted line, and invoke "Cody → Edit Code".
In the prompt, write "Remove all indents".
The result should come out correctly formatted, with the proper indents and each statement in its own line.
- But if you place a breakpoint inside FixupSession.performInlineEdits, you'll see that the edit arrives from the agent without indents (as requested), and is later formatted by IntelliJ.
Click "Undo" or "Edit & Retry".
The code will return to its original state, complete with the original formatting problem.

Things that don't work

These things seem to be broken in master.

In some cases, the selection is smaller than the modified part of the code.
If you ask Cody to edit code without selecting a range in the file, the generated code will not be selected a,d the Undo action will be broken.
Show Diff action seems to be broken in many cases.

src/main/kotlin/com/sourcegraph/cody/edit/sessions/FixupSession.kt

pkukielka

LGTM

pkukielka · 2024-05-14T11:51:13Z

src/main/kotlin/com/sourcegraph/cody/edit/sessions/FixupSession.kt

+          // when the user selects a different snippet in the UI.
+          // FIXME this causes a problem when the entire file becomes shorter than the original
+          // selection range.
+          selectionRange!!.let {


Please do not use !!

I believe that !! is justified here. However, I just confirmed that this FIXME still needs to be fixed...

src/main/kotlin/com/sourcegraph/cody/edit/sessions/FixupSession.kt

pkukielka · 2024-05-14T11:57:05Z

src/main/kotlin/com/sourcegraph/cody/edit/sessions/FixupSession.kt

+          // by calling triggerFixupAsync() which in turn calls ensureSelectionRange().
+          // The value of this property does not change
+          // when the user selects a different snippet in the UI.
+          // FIXME this causes a problem when the entire file becomes shorter than the original


Is that going to be fixed as part of this PR?

I have refactored FixupSession to use the RangeMarker from the IntelliJ API. Unlike the dumb data classes such as Range, TextRange etc., the RangeMarker remains tied to its document, and updates itself whenever the document is modified. The downside is that we need to acquire a read lock before creating a new RangeMarker.

This has eliminated the problems with shortening the file. However, I've run into other problems...

src/main/kotlin/com/sourcegraph/utils/CodyFormatter.kt

pkukielka · 2024-05-16T16:04:31Z

src/main/kotlin/com/sourcegraph/utils/CodyFormatter.kt

@@ -34,6 +34,8 @@ class CodyFormatter {
                .createFileFromText("TEMP", file.fileType, appendedString)

        val codeStyleManager = CodeStyleManager.getInstance(project)
+        // This will fail if cursor < range.startOffset + completionText.length


I think we should not crash whole edit if formatter fails.
Instead we should backoff from formatting, log the error, and continue edit without reformatting.

Actually, this is the current behaviour. However, the root cause (incorrect ranges being passed) still needs to be fixed.

…the agent

pkukielka · 2024-05-20T08:24:30Z

LGTM

Closes #1631, partially reverts #1454. ## Test plan None

odisseus requested a review from pkukielka May 6, 2024 16:38

odisseus force-pushed the mg/issue-1112 branch 2 times, most recently from a906a2f to 80b6d56 Compare May 7, 2024 11:06

odisseus self-assigned this May 7, 2024

odisseus force-pushed the mg/issue-1112 branch from 80b6d56 to fa2451c Compare May 7, 2024 12:53

pkukielka reviewed May 7, 2024

View reviewed changes

src/main/kotlin/com/sourcegraph/cody/edit/sessions/FixupSession.kt Outdated Show resolved Hide resolved

odisseus force-pushed the mg/issue-1112 branch from fa2451c to eaf2f1d Compare May 7, 2024 16:38

odisseus marked this pull request as ready for review May 8, 2024 08:02

odisseus marked this pull request as draft May 8, 2024 09:44

pkukielka approved these changes May 8, 2024

View reviewed changes

odisseus force-pushed the mg/issue-1112 branch from 91b5be8 to d8fbdb1 Compare May 9, 2024 01:27

odisseus marked this pull request as ready for review May 9, 2024 05:19

odisseus marked this pull request as draft May 9, 2024 05:29

odisseus force-pushed the mg/issue-1112 branch 3 times, most recently from 33d57cd to b21406b Compare May 13, 2024 10:42

odisseus requested a review from pkukielka May 13, 2024 13:20

odisseus marked this pull request as ready for review May 13, 2024 14:47

odisseus force-pushed the mg/issue-1112 branch from b881cc0 to 5af3c9b Compare May 14, 2024 10:28

pkukielka reviewed May 14, 2024

View reviewed changes

src/main/kotlin/com/sourcegraph/cody/edit/sessions/FixupSession.kt Outdated Show resolved Hide resolved

pkukielka reviewed May 14, 2024

View reviewed changes

src/main/kotlin/com/sourcegraph/utils/CodyFormatter.kt Outdated Show resolved Hide resolved

odisseus marked this pull request as draft May 14, 2024 14:15

odisseus force-pushed the mg/issue-1112 branch 6 times, most recently from 317a713 to 6a60cdf Compare May 16, 2024 12:13

odisseus requested a review from pkukielka May 16, 2024 13:25

odisseus marked this pull request as ready for review May 16, 2024 13:34

pkukielka reviewed May 16, 2024

View reviewed changes

odisseus added 10 commits May 20, 2024 07:24

Apply IntelliJ formatting to the edited code snippet that comes from …

fd82005

…the agent

comment about selectionRange

352b587

private

070d743

fixme

d87b7ab

Skip the formatting if the current edits are part of an undo operation.

d6380e0

set cursor offset to thebeginning of the changes

5924823

format

4e7b5f9

undoInProgress as state of FixupSession

e9b16b4

cleanup

8e13790

RangeMarker

8883375

odisseus force-pushed the mg/issue-1112 branch from 6a60cdf to 8883375 Compare May 20, 2024 05:29

odisseus merged commit bf23b92 into main May 20, 2024
6 checks passed

odisseus deleted the mg/issue-1112 branch May 20, 2024 08:39

kalanchan mentioned this pull request May 20, 2024

Inline Edits: Indent/format the rewritten code #1112

Open

odisseus mentioned this pull request May 24, 2024

Disable IntelliJ formatting for inline edits #1632

Merged

steveyegge pushed a commit that referenced this pull request May 24, 2024

Disable IntelliJ formatting for inline edits (#1632)

30e3b41

Closes #1631, partially reverts #1454. ## Test plan None

steveyegge pushed a commit that referenced this pull request May 25, 2024

Disable IntelliJ formatting for inline edits (#1632)

55dea0d

Closes #1631, partially reverts #1454. ## Test plan None

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply IntelliJ formatting to the edited code snippet that comes from the agent #1454

Apply IntelliJ formatting to the edited code snippet that comes from the agent #1454

odisseus commented May 6, 2024 •

edited

pkukielka left a comment

pkukielka May 14, 2024

odisseus May 14, 2024 •

edited

pkukielka May 14, 2024

odisseus May 16, 2024 •

edited

pkukielka May 16, 2024

odisseus May 20, 2024

pkukielka commented May 20, 2024

Apply IntelliJ formatting to the edited code snippet that comes from the agent #1454

Apply IntelliJ formatting to the edited code snippet that comes from the agent #1454

Conversation

odisseus commented May 6, 2024 • edited

Test plan

Things that are confirmed to work (prompts might need to be reworded)

Intentionally misformatting the file

Things that don't work

pkukielka left a comment

Choose a reason for hiding this comment

pkukielka May 14, 2024

Choose a reason for hiding this comment

odisseus May 14, 2024 • edited

Choose a reason for hiding this comment

pkukielka May 14, 2024

Choose a reason for hiding this comment

odisseus May 16, 2024 • edited

Choose a reason for hiding this comment

pkukielka May 16, 2024

Choose a reason for hiding this comment

odisseus May 20, 2024

Choose a reason for hiding this comment

pkukielka commented May 20, 2024

odisseus commented May 6, 2024 •

edited

odisseus May 14, 2024 •

edited

odisseus May 16, 2024 •

edited