-
Notifications
You must be signed in to change notification settings - Fork 34
Correctly interpret string arguments as booleans in electron arguments. #1536
Conversation
src/js/lib/content/src/index.js
Outdated
@@ -438,6 +439,36 @@ function ok(value) { | |||
return value !== null && value !== undefined | |||
} | |||
|
|||
/// Check whether the value is a string with value `"true"`/`"false"`, if so, return the | |||
// appropriate boolean instead. Otherwise, return the original value. | |||
function tryAsBoolean(value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the name of the function could be more descriptive, like parseBooleanOrLeaveAsIs
I know its longer, but "try" in name tells me that we are returning something like Option, but here we are returning the original value. It could also be tryAsBooleanOrLeaveAsIs
, but parse
is better as it tells that we are working with string input.
src/js/lib/content/src/index.js
Outdated
/// Turn all values that have a boolean in string representation (`"true"`/`"false"`) into actual | ||
/// booleans (`true/`false``). | ||
function ensureBooleans(config) { | ||
for (const key in config) { | ||
config[key] = tryAsBoolean(config[key]) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd call it parseAllBooleans
instead - I think this name is more descriptive.
I think this should be connected to https://github.com/enso-org/ide/issues/1512 |
I think, ideally we would convert the whole file (and all JS files bit by bit) to TypeScript and implement this properly using a |
Closed in favour of #1539 |
Pull Request Description
Previously arguments were passed to the main function as strings when using the electron app, or as booleans when using the development environment. Since
false
is a truthy value that was not further checked, this lead to wrong settings.This PR adds a conversion of
true
andfalse
to their boolean values, and adds an assertion that ensures that arguments are actually booleans.Checklist
Please include the following checklist in your PR:
CHANGELOG.md
was updated with the changes introduced in this PR.All code has automatic tests where possible.All code has been profiled where possible.All code has been manually tested in the "debug/interface" scene.