Closed aviatesk closed 4 years ago
This is good news!
You can make the functions a class so you can have multiple instances of Selector (makes code modular), and have more performance:
You wouldn't need to define global selector here: https://github.com/JunoLab/atom-julia-client/pull/722/files#diff-00780cdb3b44fe00cc8c5aa115b0f79fR8
let selector
let panel, ink
You wouldn't need to define the props every time you call show()
: https://github.com/JunoLab/atom-julia-client/pull/722/files#diff-00780cdb3b44fe00cc8c5aa115b0f79fR54
selector.props.didConfirmEmptySelection = () => {
confirmed = true
selector.cancelSelection()
const query = selector.getQuery()
if (allowCustom && query.length > 0) {
resolve(query)
} else {
resolve()
}
}
See here: https://github.com/aminya/atom/blob/266685ae3f2931eeac265211e9669793e0b3d53f/packages/line-ending-selector/lib/selector.js or https://github.com/aminya/atom-indent-detective/blob/0e85161304bf45e4bcade521646e9231c8c3a5b5/src/selector.ts#L7
You wouldn't need to define the props every time you call show():
see the callbacks take each resolve
and reject
It is possible to do these stuff purely functional, but it makes the code complex while it can be rewritten in a more readable manner.
How do you handle multiple instances of Selector?
I know it might be hard for now, maybe in later PRs we can address it, But it pays off when we use classes.
I really don't get your point. What do you mean by "purely functional" ? And why and when do we need multiple selector modals ?
I really don't get your point. What do you mean by "functional programming" ? And why and when do we need multiple selector instance ?
You don't need to import/export your global selector instance 4 times. : 1) https://github.com/JunoLab/atom-julia-client/blob/d5bb9eac37feea4e12739dca3d1b97471302a78d/lib/runtime/evaluation.coffee#L6 2) https://github.com/JunoLab/atom-julia-client/blob/d5bb9eac37feea4e12739dca3d1b97471302a78d/lib/runtime/modules.coffee#L7 3) https://github.com/JunoLab/atom-julia-client/blob/d5bb9eac37feea4e12739dca3d1b97471302a78d/lib/runtime/packages.js#L4 4) https://github.com/JunoLab/atom-julia-client/blob/d5bb9eac37feea4e12739dca3d1b97471302a78d/lib/ui.coffee#L5
The current design makes a single SelectorList instance and updates it every time it is called. This is very expensive. Imagine if you had to refresh your webpage for doing every task.
You are handling RAII manually here, while this feature is already implemented in JavaScript classes to use. You can just instantiate the class (or struct in Julia), and then call functions on it (dispatch). You don't need to check the existance of the variable manually.
selector
handler works