Combine string and function callback for button view

[ericcornelissen]

Change

The change I'm proposing allows users to specify both atom commands and function for callback when multiple callbacks are specified. So, before it was only possible to do:

toolBar.addButton({
  type: "button",
  icon: "chevron-right",
  callback: ['application:cmd-1', 'application:cmd-2'],
  tooltip: "Show project"
})

But not with a command and function, like:

toolBar.addButton({
  type: "button",
  icon: "chevron-right",
  callback: ['application:cmd-1', () => console.log("Hello world!")],
  tooltip: "Show project"
})

With this Pull Request that is now possible.

Implementation

I changed the "executeCallback" function so that the user can use both strings (for atom command dispatching) and function (for whatever the user wants) when the callback option is set to an array.

To achieve this I convert the callback value to an array if it is not and then iterate over the array and perform either the dispatch or function call as it happened before.

Testing

I did not find any tests for this function so I also didn't add a new test.

[suda]

Thanks Eric! I see the tests fail on Beta channel for MacOS: https://travis-ci.org/suda/tool-bar/jobs/570771960 I guess that's Atom's fault. If you have a Mac, could you try running them and see it that's the case?

[ericcornelissen]

@suda, looking at the error logs it say the following for every failing test:

Expected promise to be resolved, but it was rejected with: Cannot read property 'provideToolBar' of null {  }

This is, I'm guessing, referring to this (the only other provideToolBar is here):

https://github.com/suda/tool-bar/blob/e1065dbf1aac8e6dfee964758d386deebd5ecb98/spec/tool-bar-spec.js#L35-L38

So, it seems to me that the build that's failing had some problems activating the package (can't imagine that is due to my changes). Rerunning it might make the build work again.

^{Either way I don't have a Mac to test this on}

[ericcornelissen]

Anything that is preventing this from being merged @suda? If it is the failiing build I (again) suggest to restart the build.

Also, as I said earlier I'm will to help out maintaining the package 😉

[suda]

Oh sorry, Eric! This got buried under other stuff 😓 Merging it in 👍

[ericcornelissen]

Oh sorry, Eric! This got buried under other stuff 😓 Merging it in 👍

Not a problem at all, just checking in 😉