Multiple column breakpoints that get folded together remain "duplicated"

[alexdima]

Testing #25194

Have the following source code:

function fib(n) { if ((n === 1) || (n === 0)) { return 1; } else { return fib(n - 1) + fib(n - 2); } }
console.log(fib(10));

Use Shift+F9 to add N breakpoints, in between each letter on the fib line (about 100). Use https://nodejs.org/download/nightly/v8.0.0-nightly201703249ff7ed23cd/

Press play without a debug configuration. Notice that the breakpoints have been folded together in the editor, and pressing F5 acts as if there would be a single breakpoint at a position, but the breakpoint list shows many "duplicated" breakpoints:

[isidorn]

I guess we could remove duplicate breakpoints (those that have the same line & column) however should this be done on the adapter side or on the vscode side.
Feels to me like vscode should do this so that each adapter should not worry about this. However I am not sure if by doing this we are limiting some scenario

fyi @weinand

[roblourens]

I don't think the adapter can remove breakpoints. It can only return a location for them. It would be nice for VS Code to de-dupe but we have to think about the UX in the case where a user sets a BP in some location, and the adapter returns a location that already exists. It could look like the BP just disappeared or failed to be set. When we show a dupe in the bp list, the user can figure out what happened, although it's still not really obvious. This happens a lot when setting multiple column BPs on a line.

[isidorn]

After discussing with @weinand we have two models of handling duplicate brekapoints

1. We always show also the intent of the user and the real position, thus there would be ne duplicates in this case and they would all be unique
2. We never show the intent of the user and only show the real position. This is what we are doing now and in this case it makes sense to automatically remove duplicate breakpoints since we are ignoring the "intent" of the user.

We decided to go with the second approach for now and if we see users complaing we can reconsider to go back to the first option

[roblourens]

What will they see when a breakpoint is set, and it's moved to the same location as another? If it looks like nothing happened, that might be confusing.

[isidorn]

@roblourens There will be a slight flicker after which it looks like nothing happened. Yes, this might be confusing but I suggest we wait for bugs from users.