feat: use getShellStartCommand to get the default command

[aminya]

On Windows, as the default value for the config, this uses pwsh (PowerShellCore) if available on a system. If not it will use powershell, and in the end, it will use cmd.

[the-j0k3r]

This looks good except for missing updated tests otherwise 👍

[aminya]

I added a sync version using which which is faster. Then I used a trick to work around Atom not supporting async config reading and made the config setting async.

[UziTech]

I added a sync version using which which is faster.

My tests show that if the user doesn't have powershell or pwsh it took my computer 320-380 ms to get the defaults. Without this PR it took 0.18 - 0.32 ms

I'm thinking that ~300ms is not worth the more accurate default.

If you can get Atom to load the config asynchronously it might be worth it.

[aminya]

@UziTech For the tests, could you make which function work the same way you faked childProcess?

[UziTech]

@UziTech For the tests, could you make which function work the same way you faked childProcess?

You can only mock properties and methods of an object. If you want to mock an imported function you will have to inject a spy some how.

something like:

async function getShellStartCommand (find = which) {
  ...
  find('pwsh')
  ...
}
...
it('test', () => {
  const spy = jasmine.createSpy('which')
  getShellStartCommand(spy)
  ...
})

[aminya]

This only needs the tests. If you can help with that, I would appreciate it.

[UziTech]

Should we also observe autoCommand and reset command if it is checked?

[UziTech]

Suggested change

	defaultProfile: configDefaults.command,
	defaultProfile: configDefaults.autoCommand,

[github-actions]

🎉 This issue has been resolved in version 11.0.0 🎉

The release is available on:

• GitHub release
• Atom package

Your semantic-release bot 📦🚀