Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug:The datas of Complete exists repeatedly #6130

Closed
wants to merge 1 commit into from
Closed

fix bug:The datas of Complete exists repeatedly #6130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/plugin-ext/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"request": "^2.82.0",
"uuid": "^3.2.1",
"vscode-debugprotocol": "^1.32.0",
"vscode-textmate": "^4.0.1"
"vscode-textmate": "^4.0.1",
"ts-md5": "^1.2.2"
},
"publishConfig": {
"access": "public"
Expand Down
3 changes: 3 additions & 0 deletions packages/plugin-ext/src/main/browser/languages-main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ export class LanguagesMainImpl implements LanguagesMain {
}

$registerCompletionSupport(handle: number, selector: SerializedDocumentFilter[], triggerCharacters: string[], supportsResolveDetails: boolean): void {
if (this.disposables.has(handle)) {
return;
}
this.disposables.set(handle, monaco.modes.SuggestRegistry.register(fromLanguageSelector(selector)!, {
triggerCharacters,
provideCompletionItems: (model: monaco.editor.ITextModel,
Expand Down
24 changes: 21 additions & 3 deletions packages/plugin-ext/src/plugin/languages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import { ColorProviderAdapter } from './languages/color';
import { RenameAdapter } from './languages/rename';
import { Event } from '@theia/core/lib/common/event';
import { CommandRegistryImpl } from './command-registry';
import { Md5 } from 'ts-md5/dist/md5';

type Adapter = CompletionAdapter |
SignatureHelpAdapter |
Expand Down Expand Up @@ -171,8 +172,25 @@ export class LanguagesExtImpl implements LanguagesExt {
});
}

private addNewAdapter(adapter: Adapter): number {
const callId = this.nextCallId();
private hashCode(str: string): number {
let hash: number = 5381;
let length: number = str.length;
while (length) {
hash = (hash * 33) ^ str.charCodeAt(--length);
}
return hash >>> 0;
}

private nextNewCallId(selector?: theia.DocumentSelector, triggerCharacters?: string[]): number {
let strBuffer: string = JSON.stringify(selector);
if (triggerCharacters) {
triggerCharacters.forEach((triggerCharacter: string) => strBuffer += triggerCharacter);
}
return this.hashCode(Md5.hashStr(strBuffer) as string);
}

private addNewAdapter(adapter: Adapter, selector?: theia.DocumentSelector, triggerCharacters?: string[]): number {
const callId = selector ? this.nextNewCallId(selector, triggerCharacters) : this.nextCallId();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain what is supposed to fix and how?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a need to create multiple duplicate adapters for scenes with the same selector and triggerCharacters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 different vscode extensions contribute different providers for the same selector and trigger characters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This situation is extreme but still exists, such as: vscode intellicode and redhat-java

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand an issue with that. Each provider should be handled independent and has own id.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to do something specific for this particular scenes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If deduplication is done at the data level, there is bound to be loss in overall performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akosyakov
My solution is:
If the selector and the triggerCharacters are the same, then only one provider is provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, this problem also exists in vscode, I think this is a flaw in the design of vscode.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should file a bug against vscode intellicode and redhat-java, and ask whether they supposed to be used together. It does not seem like that to me.

Code wise changes don't look good, it will lead to issues in other cases.

this.adaptersMap.set(callId, adapter);
return callId;
}
Expand Down Expand Up @@ -229,7 +247,7 @@ export class LanguagesExtImpl implements LanguagesExt {
}

registerCompletionItemProvider(selector: theia.DocumentSelector, provider: theia.CompletionItemProvider, triggerCharacters: string[]): theia.Disposable {
const callId = this.addNewAdapter(new CompletionAdapter(provider, this.documents, this.commands));
const callId = this.addNewAdapter(new CompletionAdapter(provider, this.documents, this.commands), selector, triggerCharacters);
this.proxy.$registerCompletionSupport(callId, this.transformDocumentSelector(selector), triggerCharacters, CompletionAdapter.hasResolveSupport(provider));
return this.createDisposable(callId);
}
Expand Down