Change --firefox-binary to --firefox

[kumar303]

web-ext run --firefox=/path/to/firefox-bin seems more concise. We should also give it a short alias like -f.

[kumar303]

we should fix #169 first to avoid confusion when users upgrade their web-ext for this option change

[shubheksha]

For this issue, I need to modify this line & add an alias property like it has been included for other commands -- am I on the right track?

[kumar303]

Yes, I think that's the way to go.

[shubheksha]

I made the required changes & they're being reflected using the --help option. However, I am unable to test it & running into the following error. I've attached the screenshot for reference.

[kumar303]

You should be able to use any of these extensions to test it: https://github.com/mdn/webextensions-examples/ You will need to cd into the source directory of one of the examples before executing web-ext run.

[kumar303]

One thing that may not be obvious is that yargs has some magic that was turning --firefox-binary into the config key firefoxBinary when executing run(). The run() signature probably also needs updating.

[shubheksha]

I am a little confused -- the variable firefoxBinary is strewn all over in this file. What about those?

[kumar303]

Let's say you typed:

web-ext run --firefox-binary /Applications/FirefoxNightly.app/Contents/MacOS/firefox-bin

This would invoke:

cmd.run({firefoxBinary: '/Applications/FirefoxNightly.app/Contents/MacOS/firefox-bin', ...});

The variable is passed around to a few things that utilize the Firefox executable path.

[shubheksha]

Okay, that makes sense. But I don't really understand the flow even now.
As you mentioned above, the config key firefoxBinary was being created by yargs on its own, so changing the option to --firefox should also work in a similar manner? I am sorry if this sounds stupid -- I am a little lost. I went through the yargs docs as well, but didn't find an answer to how this is being done.
From what I understood, --firefox will be changed to firefox, but that variable is already present in the file in multiple places.

[kumar303]

From what I understood, --firefox will be changed to firefox,

Yes, that's correct. It's not very well documented in yargs but if you log the argv variable you could see the complete object after yargs is done doing the magic translation.

but that variable is already present in the file in multiple places.

Ah, good point. I see the problem now. There is already an option defined as firefox=defaultFirefox so the new yargs generated firefox variable will conflict. I think you'll need to rename this everywhere that it's used. Something like firefoxApp=defaultFirefox probably makes sense.

[shubheksha]

Firefox can't be found for some reason. Changing the variable names broke something, I guess. Despite giving it the full path to the .exe, it won't work. Hence, the builds are failing too. How can I go about finding what exactly broke?

[kumar303]

The run command docs will need updating but we should wait until a new version of web-ext is released containing this change.

[kumar303]

I updated the docs