microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
161.62k stars 28.39k forks source link

Debugging should handle identical sources in sourceReference better #85290

Open DanTup opened 4 years ago

DanTup commented 4 years ago

I'm trying to improve the debugging experience for Flutter when using Hot Reload. There are a few places where the behaviour is currently confusing:

  1. The user modifies a file in the editor and then hits a breakpoint - the file contents on screen do not match what the VM is running
  2. The user modifies a file and saves, which results in new breakpoints being sent to the VM with the new locations - however if the VM was not hot reloaded, we just moved all the breakpoints in incorrect locations
  3. Even after a hot reload, it's possible for the VM to pause in a location based on old source code (currently it will take the user to the new source code in the editor, which is not what's actually being executed)

I thought many of these issues could be resolved by using stackTraceRequest in the getStackTrace call to give the real source code back to VS Code. This works reasonable well when the source codes are different (eg. the user modified a file and saved, but we did NOT hot reload). They end up getting a new editor window showing the source code we're executing.

However, if the source files are identical, the debug experience is poor. VS Code still opens a new editor window (even though the contents are the same as what they already had open) and:

  1. This editor is read-only. This prevents the user from making changes to then hot-reload
  2. VS Code treats this as a completely different file (even though the paths are the same) and therefore it does not show any of the breakpoints that were set, and adding new breakpoints will result in a "different list" being sent to the debug adapter which is very confusing.

For example here, these two files are the same - the one on the left is the "real" file and the one on the right is the sourceReference-provided file (but with the same path). Notice they have different breakpoints (which cannot be distinguished in the Breakpoints list):

Screenshot 2019-11-21 at 10 40 10 am

Screenshot 2019-11-21 at 10 41 10 am

If you toggle breakpoints in these files, the DA gets these requests:

{
    "source": {
        "name": "bp_testing.dart",
        "path": "/Users/dantup/Desktop/Dart Sample/bin/bp_testing.dart",
        "sourceReference": 2,
    },
    "breakpoints": [
        {
            "line": 5
        }
    ],
    "sourceModified": false
}

{
    "source": {
        "name": "bp_testing.dart",
        "path": "/Users/dantup/Desktop/Dart Sample/bin/bp_testing.dart"
    },
    "breakpoints": [
        {
            "line": 2
        },
        {
            "line": 7
        }
    ],
    "sourceModified": false
}

This is confusing, because if the DA just passes the breakpoints requests on, it will flip-flop between the breakpoints depending on which file was used to set a breakpoint last.

I think VS Code could be smarter when it gets a sourceReference file with a path that matches a file on disk exactly. It could open the normal editor for that file (so there is only one set of breakpoints, and the file is editable). If the user makes changes and hits step, then on the next request the contents would not match, so VS Code would open the new/different editor (which is readonly, as it represents the contents in the VM). When this new editor is opened, it should also show the breakpoints that were last sent to the DA (currently it shows nothing).

I don't think this is solvable by the DA (eg. conditionally using sourceReference depending on whether the file is different) because the DA has no access to know whether the file is the same in the VM as the one the user sees in the editor.

vscodebot[bot] commented 4 years ago

(Experimental duplicate detection) Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

DanTup commented 4 years ago

@weinand I just found there's a checksum field in the Source object in the DAP, but I can't find any explanation for what it's used for (I've checked for issues and PRs both here and the DAP repo). Would that help here? Is there any info on it?

roblourens commented 1 year ago

@weinand @isidorn I also don't see much about the checksums, I assume it's something that VS uses, any idea?

isidorn commented 1 year ago

@roblourens I do not know what VS uses.