DAP sources are case sensitive on insensitive filesystems

[connor4312]

From #102493 (comment)

In this case:

1. The user had Node.js importing a different casing of their file (socks5udp) than what was actually on their disk (socks5UDP).

{
  "source": {
    "name": "socks5UDP.js",
    "path": "c:\\Users\\Rohit Kapoor\\source\\sockem\\lib\\socks5UDP.js"
  },

2. They set breakpoints, VS Code set in socks5UDP
3. We set the breakpoint, and V8 resolved it. V8 used the imported casing socks5udp
4. Then js-debug sent back that the breakpoint in the source, using the casing/path from Chrome

{
  "name": "lib/socks5udp.js",
  "path": "c:\\Users\\Rohit Kapoor\\source\\sockem\\lib\\socks5udp.js",
  "sourceReference": 0
}

5. This caused the breakpoint to move out of its correct file in VS Code

What should be the proper behavior here? Who should canonicalize the path?

[weinand]

Yes, the fact that node.js allows to import a different casing of a file (and the fact that drive letter casing on Windows is not predictable either) is a nightmare.

[In the legacy node-debugger I was registering breakpoints with a regexp instead of path for that reason (to make the breakpoint matching case insensitive). But it seems that breakpoint matching is not your problem here...]

We came to the conclusion that ideally VS Code should handle cases like this in a smart way: mapping file paths that differ only in casing back to a single file in the workspace (instead of opening another editor...).

However, at that time for some reason (@isidorn?) it was not easy/possible to implement that strategy with the workspace API. So we added workarounds in the debug adapters to return paths that would "match" VS Code's expectations.

But today workspace has better support for smart matching files with case differences. I think we should finally tackle the problem on the VS Code side so that debug adapters don't have to deal with this anymore.

@isidorn what is your take?

[bpasero]

Fyi @jrieken and me tackled casing issues in the workbench with the use of IUriIdentityService and specifically asCanonicalUri in dedicated places e.g. where we open an editor or where we receive markers that we need to correlate back to an opened editor.

[isidorn]

I think it makes sense that we tackle this on the VS Code. More specificily:

• Whenever VS Code ui recieves a Source object we would always use the asCanonicalUri internally.
• Open question is what should VS Code send out to the Debug Extension when sending back that Source object. Probably the same form that VS Code recieved from the debug extension

Let me know what you think

[connor4312]

Open question is what should VS Code send out to the Debug Extension when sending back that Source object. Probably the same form that VS Code recieved from the debug extension

At least for js-debug, I always normalize path casing on windows, so it doesn't matter.

[isidorn]

Ok, since there are no complaints let's do what I suggested above.
Thus assigning to me and September.

[weinand]

@isidorn Yes, VS Code should leave data received from the DA unmodified when sending it back to the DA.

[isidorn]

I have pushed a change as we have agreed Whenever VS Code ui recieves a Source object we would always use the asCanonicalUri internally. And VS Code leaves data from the DA unmodified.

However I did not have a chance to try this out since I do not have exact repro steps.
@connor4312 can you please try it out with VS Code insiders from tomorrow or can you share exact repro steps so I try this on my machine?

[isidorn]

Insiders build with this change is out. Let me know how it behaves. Thanks!

[connor4312]

I was able to repro this in today's insiders. Here's what I'm doing: https://memes.peet.io/img/20-09-8e9d658e-4062-4eca-9e66-068c3ae4c21a.mp4

[isidorn]

@connor4312 thanks a lot for that example. I was able to repro. Required a bit more changes from my side, but now it behaves good for me.
So closing this.
If you see more issues please let me know, though now I am confident about the change.

[isidorn]

Let's add a verification needed here. So @connor4312 can add verified when this seems to work all right for him.

[connor4312]

Looks good in my testing 👍

[isidorn]

Thanks a lot for testing it out!