microsoft / vscode-pull-request-github

GitHub Pull Requests for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github
MIT License
2.29k stars 574 forks source link

Generated code suggestion can't be applied to the file correctly #6171

Open rebornix opened 2 weeks ago

rebornix commented 2 weeks ago

Testing #6160

Initial code

preserveElements(indexes: number[]): void {
    this.preservedItems.clear();
    for (const index of indexes) {
        if (index >= 0 && index < this.items.length) {
            const item = this.items[index];
            this.preservedItems.add(item);
            if (!item.row) {
                this.insertItemInDOM(index);
            }
            this.updateItemInDOM(item, index);
        }
    }
}

new code

preserveElements(indexes: number[]): void {
    this.preservedItems.clear();
        let updatePreservedElements = false;
    for (const index of indexes) {
        if (index >= 0 && index < this.items.length) {
            const item = this.items[index];
            this.preservedItems.add(item);
            if (!item.row) {
                this.insertItemInDOM(index);
                updatePreservedElements = true;  
            }
            this.updateItemInDOM(item, index);
        }
    }

    if (updatePreservedElements) {  
            this.rerender();  
        }  
}

The generated code suggestion

        let updatePreservedElements = false;  
        for (const index of indexes) {  
            if (index >= 0 && index < this.items.length) {  
                const item = this.items[index];  
                this.preservedItems.add(item);  
                if (!item.row) {  
                    this.insertItemInDOM(index);  
                    updatePreservedElements = true;  
                }  
            }  
        }  

        if (updatePreservedElements) {  
            this.rerender();  
        }  

However when applied to the editor, the file was in a broken state

image
alexr00 commented 2 weeks ago

This has to do with the re-positioning of comments when the file changes.

alexr00 commented 2 weeks ago

Here's one of the things that can happen:

  1. User opens a file and makes a comment on a line.
  2. The line position changes because of lines added before the comment. VS Code updates where the decoration is shown, but the line number of the comment in the comment API stays the same.
  3. At this point, everything looks visually correct, but the comment on the extension host no longer matches the line number that's used in the renderer.
alexr00 commented 2 weeks ago

In VS Code core, we're going to need to propagate range changes back to the extension.

alexr00 commented 2 weeks ago

Making this change could break extensions if they're using the VS Code CommentThread API as their data model. I'll need to discuss at the API sync. @chrmarti FYI.

alexr00 commented 2 weeks ago

Thinking about it some more, I think each extension should handle this. Each extension might have a different idea of what file reversion is original. There is also a bug in VS Code that I'll fix, but otherwise, each extension should maintain a mapping of their comment's location to a location in a text document.

chrmarti commented 1 week ago

I think it would help extensions if VS Code just moved comments along as lines are added / removed. Are you saying you would need the previous revision to restore the correct position after the window reloads because the updated positions are not stored?

alexr00 commented 1 week ago

I'm not sure it's possible for VS Code to know where the right position is.

  1. An extension knows that it should have a comment on line 3 based on revision A
  2. Some revisions happen to the file, and now locally we're on revision C.
  3. The extension knows what a revision means to it, and a revision could mean different things to different extensions (git commits, difference from file on disk, changes since the file was opened).
  4. Using this knowledge, when the file is opened in VS Code, the extension can map the position in revision A to the position in revision C. If this is the first time the file has been opened in VS Code with these comments, then VS Code has no understanding of how to map a position in A to a position in C because it doesn't understand what A and C are.

GHPR already does step 4, and presumably other comment providing extensions do the same.

What VS Code core should definitely do is move the comment based on active editing of the file. Ex., there's a comment on line 3 and you edit the file in VS Code and insert 2 lines above that: the comment should move to line 5.