Skip to content

Removed a redundand 'resolve' in SimpleFileDialog by reusing the alre…#99432

Merged
alexr00 merged 5 commits intomicrosoft:masterfrom
stoyannk:dialog_fix
Jun 9, 2020
Merged

Removed a redundand 'resolve' in SimpleFileDialog by reusing the alre…#99432
alexr00 merged 5 commits intomicrosoft:masterfrom
stoyannk:dialog_fix

Conversation

@stoyannk
Copy link
Contributor

@stoyannk stoyannk commented Jun 5, 2020

…ady queried stat data.

This PR fixes #99384

Removed the redundant call to 'resolve' when creating items in the CimpleFileDialog by reusing the stat data already available for each item.

@ghost
Copy link

ghost commented Jun 5, 2020

CLA assistant check
All CLA requirements met.

let fullPath = resources.joinPath(parent, stat.name);
if (stat.isDirectory) {
fullPath = resources.addTrailingPathSeparator(fullPath, this.separator);
return { label: resources.basename(fullPath), uri: fullPath, isFolder: true, iconClasses: getIconClasses(this.modelService, this.modeService, fullPath || undefined, FileKind.FOLDER) };
Copy link
Member

Choose a reason for hiding this comment

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

Still should do resuorces.basename(fullPath) before the addTrailingPathSeparator since you can end up with an extra path separator.

@alexr00 alexr00 added this to the June 2020 milestone Jun 5, 2020
@alexr00 alexr00 added the simple-file-dialog Issues with simple file dialog label Jun 5, 2020
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

Good PR, thank you for improving the performance! We are currently getting ready for our May release, so I won't merge this PR immediately. Once we have branched for release I will merge it.

@stoyannk
Copy link
Contributor Author

stoyannk commented Jun 5, 2020

Thank you for the fast review!

@alexr00 alexr00 merged commit d57a038 into microsoft:master Jun 9, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

simple-file-dialog Issues with simple file dialog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

simpleFileDialog unnecessarily resolves files multiple times

2 participants