search

WICG / file-system-access

Expose the file system on the user’s device, so Web apps can interoperate with the user’s native applications.
https://wicg.github.io/file-system-access/
Other
654 stars 65 forks source link

Rename `excludeAcceptAllOption` to `showAcceptAllOption` #415

Closed AshleyScirra closed 1 year ago

AshleyScirra commented 1 year ago

The excludeAcceptAllOption uses negative naming, which results in double negatives, which are a bit of a headache in API design.

For example: want to show the 'accept all' option? You need excludeAcceptAllOption: false, which is a double negative. This might not seem like much of an issue as that's the default and so will typically be used as excludeAcceptAllOption: true. However sometimes it's controlled dynamically e.g. excludeAcceptAllOption: !showAcceptAll in which case you have to think about the inverted logic.

In general I think it's always better to use positive naming only, i.e. showAcceptAllOption that defaults to true. If you don't want to show it then you specify showAcceptAllOption: false.

mkruisselbrink commented 1 year ago

Optional booleans that default to true are generally strongly discouraged since they lead to the unfortunate situation where undefinedAPIs that have boolean arguments defaulting to true (see "APIs that have boolean arguments defaulting to true" link in https://w3ctag.github.io/design-principles/#prefer-dictionaries). Although I agree that the double negatives aren't great either.

a-sully commented 1 year ago

I agreed that the naming is unfortunate, but as @mkruisselbrink described above I'm not sure if there's a better option :/

gplanansky commented 1 year ago

"exclude" induces cognitive dissonance.
Use "hide" : "hideAcceptAllOption".
"hide" is a straightforward action verb a three year old groks. So the default value of false happily makes sense to humans; and, happily also avoids the undefined implementation pitfall.