Skip to content

Conversation

KazariEX
Copy link

No description provided.

@KazariEX
Copy link
Author

KazariEX commented Jul 15, 2025

Hi @aeschli, sorry to bother you, this is a very simple fix, would you like to have a look?

@aeschli aeschli added this to the September 2025 milestone Sep 29, 2025
@aeschli
Copy link
Collaborator

aeschli commented Sep 29, 2025

Thanks @KazariEX !

@aeschli aeschli enabled auto-merge (squash) September 29, 2025 15:40
@aeschli aeschli closed this Sep 29, 2025
auto-merge was automatically disabled September 29, 2025 16:11

Pull request was closed

@aeschli aeschli reopened this Sep 29, 2025
@aeschli aeschli modified the milestones: September 2025, Backlog Sep 29, 2025
@aeschli
Copy link
Collaborator

aeschli commented Sep 29, 2025

See the test failure. We don't want duplicated proposals. The user would not know which one to choose.
If you want to replace a proposal, the first one needs to be replaced.
But I don't think e can make guarantees on the order of the providers

@KazariEX
Copy link
Author

Do you think we should move the processing of global attributes up front and deduplicate the attributes here?

provideAttributes(tag: string) {
const attributes: IAttributeData[] = [];
const processAttribute = (a: IAttributeData) => {
attributes.push(a);
};
const tagEntry = this._tagMap[tag.toLowerCase()];
if (tagEntry) {
tagEntry.attributes.forEach(processAttribute);
}
this._globalAttributes.forEach(processAttribute);
return attributes;
}

@KazariEX
Copy link
Author

See the test failure. We don't want duplicated proposals. The user would not know which one to choose. If you want to replace a proposal, the first one needs to be replaced. But I don't think e can make guarantees on the order of the providers

I think we can make it so that this situation doesn't occur by default, but we should not completely prevent extensions from providing properties with the same name if they want to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants