fix default Anchor param of context menu

[shuyaqian]

Signed-off-by: shuyaqian 717749594@qq.com

What it does

Fix default Anchor param of context menu.

How to test

1. click 'Start Debugging' menu
   
2. you will get error
   
3. because the param debugSessionOptions received is an anchor
   

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[colin-grant-work]

A similar change has been proposed previously, but it was rejected because there was code that relied on the current behavior. I believe that this recent change should reduce our dependence on that behavior upstream, but there may be quite a lot of downstream code that still relies on it.

Given that, there are other possibilities for working around this problem in the current context.

• Modify the handler for the command that is failing to account for the possibility of an anchor as the first argument.
• Modify the code that opens that menu to explicitly pass undefined as the first argument (or first several arguments)

[shuyaqian]

A similar change has been proposed previously, but it was rejected because there was code that relied on the current behavior. I believe that this recent change should reduce our dependence on that behavior upstream, but there may be quite a lot of downstream code that still relies on it.

Given that, there are other possibilities for working around this problem in the current context.

• Modify the handler for the command that is failing to account for the possibility of an anchor as the first argument.
• Modify the code that opens that menu to explicitly pass undefined as the first argument (or first several arguments)

I defined the isAnchor function, and verify the first argument.

[msujew]

Suggestion: We try to limit our any usage in the framework.

Suggested change

	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	export function isAnchor(arg: any): boolean {
	if (arg && 'x' in arg && 'y' in arg && Object.keys(arg).length === 2) {
	export function isAnchor(arg: unknown): boolean {
	if (typeof arg === 'object' && arg && 'x' in arg && 'y' in arg && Object.keys(arg).length === 2) {

[colin-grant-work]

It would be better to check this condition in the command handler (around line 640), since the possibility of receiving of anchor is limited to that context.

[shuyaqian]

It would be better to check this condition in the command handler (around line 640), since the possibility of receiving of anchor is limited to that context.

ok， done

[colin-grant-work]

Is this correct and sufficient? The Anchor type also includes MouseEvents (l. 26).

export interface Coordinate { x: number; y: number; }
export const Coordinate = Symbol('Coordinate');
export type Anchor = MouseEvent | Coordinate;

That also seems to be the error that is killing the build.

[shuyaqian]

I changed the type to unknown.

[colin-grant-work]

Sorry to change my mind here, but even though the cause of the problem is that an Anchor is being passed in, but what we really want to guarantee is that the config is an instance of DebugSessionOptions - the problem with the CommandHandler system is that we're allowed to say config?: DebugSessionOption, but in fact anything can be passed in - Anchors, or anything else..

So rather than adding a check for whether this is an Anchor, it would be more correct in this context to implement a check that something is a DebugSessionOptions. Something like.

export namespace DebugSessionOptions {
    export function is(candidate: unknown): candidate is DebugSessionOptions {
        const options = candidate as DebugSessionOptions;
        return typeof options === 'object' && !!candidate && 'configuration' in candidate && DebugConfiguration.is(options.candidate);
    }
}

Again, sorry for the change of direction!

[colin-grant-work]

I would say hold off on this pending a general resolution of the problems introduced by window.menuBarVisibility compact. As I say here, the correct, non-breaking solution is probably just to fix the behavior of the menu introduced by that PR, rather than fix the commands one by one or change the existing behavior of real context menus.