Could you explain more about this method? The return type isthis, but the actual returned value istrue

param is type is a user-definedtype guard in typescript's grammar.

It reduced the scope of union type:

functionisTypeDefined(obj:Type|undefined):obj isType{returnobj!==undefined;}leta:Type|undefined;a.method()// ts error, a may be undefinedif(isTypeDefined(a)){a.method();// now a's type is only `Type`, `undefined` is reduced}

this is type is a way to guard a class member's type:
microsoft/TypeScript#26212

this is { panel: WebviewPanel } makes surethis.panel is not undefined.

So the code here do not need to usethis.panel!.method() to assert defined:

if(this.showWebviewInternal()){this.panel.title=`${node.name}: Preview`;// now this.panel's undefined type is reducedthis.panel.reveal(ViewColumn.One);}

Then why we are not just callingshowWebviewInternal() but put it in the if block.

I do not see any benefits to do this.

You mean the code below?

this.showWebviewInternal();this.panel!.title=`${node.name}: Preview`;// this.panel's undefined type is not reducedthis.panel!.reveal(ViewColumn.One);

The main problem here is, we should extract the fields inILeetCodeWebviewOption as the member of the base class. The reason for doing that:

1. Each web view has its own uniqueviewType
2. Each web view has its own location to display - theviewColumn
3. Each web view has the member title to determine the title of the editor.

So after the extraction, what you need to do is:

1. Move all the logic about the panel update intoshowWebviewInternal()
2. Update the title field when needed.

this.title=xxxxthis.showWebviewInternal();

I've brought everything exceptsideMode in next PR to this PR. It includes the improvement on the webview base class, like mentioned above.

Do you really think there is no relationship? Did you ever test yourself what is the visual effect when the title keepsA so long long long long long name: preview andA so long long long long long name: solution in Column 2? Will the user be able to easily change between the tabs?

Why do you pay more attention to something in the next PR which I haven't given illustration yet?

I just want leetcode tabs in column 2 look like this way:

Otherwise, in my small-screen PC, aSo long long long a name: Preview just occupies all the room in the Column 2.

Of cause I need to understand what you will do in the next PR since they have some relationship. This will determine whether we need to keepgetWebviewOption() or make them as the class member.

The codes written in a good way should not only have good experience but also good at self-explain. This is very important to attract more community contribution. That's why I'm keeping asking you those questions. From the code itself, I cannot get your point. There's no GIF/picture to illustrate that. Then how could I know your point is that the tab is so long that make the experience bad?

Besides, From my understanding, the reason to rename to the title toDescription is that you hope the size of the tab is not too big when VS Code has two columns. The real reason actually is having limited resolution, and multiple columns is one of the possible reason to cause the limited resolution.

Take the VS Code embedded Markdown extension as an example, You can see there is no optimization when the tab is too long.

I'm not saying that we should not do the optimation. It's acceptable to do it as long as the code logic is easy to understand.

nit: TheonDidReceiveMessage can also be disposed inonDidDisposeWebview

Put it intosubscriptions is fine though.

another alternative is to passthis.context.subscriptions toworkspace.onDidChangeConfiguration, and do not maintainconfigListener inLeetCodeWebview?

Just my personal opinion: putting intosubscriptions is a passive way to clean up the memory:

An array to which disposables can be added. When this extension is deactivated the disposables will be disposed.

It's more suitable for the command, global objects, etc. Since once the command is registered, in most cases you will not remove it during the extension is running.

While here, once the panel is disposed. We need to dispose the listener as well to avoid that multiple listeners are working in the meantime. So disposing them in theonDidDisposeWebview() could be better.

This way?

private listeners:Disposable[]=[];//...this.panel.onDidDispose(this.onDidDisposeWebview,this,this.listeners);this.panel.webview.onDidReceiveMessage(this.onDidReceiveMessage,this,this.listeners);workspace.onDidChangeConfiguration(this.onDidChangeConfiguration,this,this.listeners);//...protectedonDidDisposeWebview():void{    this.panel=undefined;for(constlistenerofthis.listeners){listener.dispose();}this.listeners=[];}

In this way,this.context will reportnever used by tslint. We could make such modification:

publicinitialize(context:ExtensionContext):void{    this.context=context;this.context.subscriptions.push(this);}

It separates the intialization code inactivate function. Or, we could just movethis.context out of webview.

just movethis.context out of webview looks good to me.

Then why we are not just callingshowWebviewInternal() but put it in the if block.

I do not see any benefits to do this.