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.

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

chooseFileSystemEntries naming and behavior is misleading #146

Closed
tomayac opened this issue Jan 22, 2020 · 2 comments
Closed

chooseFileSystemEntries naming and behavior is misleading #146

tomayac opened this issue Jan 22, 2020 · 2 comments
Milestone

Comments

@tomayac
Copy link
Contributor

@tomayac tomayac commented Jan 22, 2020

The method name chooseFileSystemEntries suggests it's for choosing files (plural), which then in turn implies the multiple field from the ChooseFileSystemEntriesOptions is somewhat superfluous.

Do we maybe need a new method chooseFileSystemEntry (singular) and consequently remove the multiple field from the ChooseFileSystemEntriesOptions entirely?

@mkruisselbrink
Copy link
Contributor

@mkruisselbrink mkruisselbrink commented Apr 9, 2020

I have no strong opinions on naming here. Originally I had chooseFileSystemEntries always return its results as an array, even if multiple was set to false. But always having to extract an item from that array if you're only expecting a single item anyway seemed annoying, hence why it was changed to not wrap the result in an array if multiple is false. I do agree that that resulted in somewhat unfortunate naming.

As an aside, I'm not a big fan of the chooseFileSystemEntries name at all, perhaps we can side-step this issue with a completely different name? showFilePicker (but what about directories?), or something else? Of course that would still leave the awkwardness that the return type of the method is determined by a passed in option (but that is also true for file vs directory).

It does seem like it might be cleaner to have separate methods with well-defined return types instead. One extreme might be to have 3 (or 4) different methods for all the different possible return types, i.e. separate chooseFile, chooseFiles and chooseDirectory methods (the 4th would be chooseDirectories, but at least chrome doesn't actually support that today; {type: 'open-directory', multiple: true} actually only lets you pick a single directory, the result just gets wrapped in an array).

@mkruisselbrink mkruisselbrink added this to the V1 milestone Apr 9, 2020
@mkruisselbrink
Copy link
Contributor

@mkruisselbrink mkruisselbrink commented Apr 9, 2020

Also sort of a duplicate of #25. So I think I'll close this one in favor of the earlier issue.

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

No branches or pull requests

2 participants