Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracker: Set defaults for all options using a config file #176

Open
kumar303 opened this issue Apr 6, 2016 · 11 comments
Open

Tracker: Set defaults for all options using a config file #176

kumar303 opened this issue Apr 6, 2016 · 11 comments

Comments

@kumar303
Copy link
Contributor

kumar303 commented Apr 6, 2016

If a user has a magically named config file in the current working directory (e.g. web-ext-config.json) then web-ext should use this to set default option values (and web-ext should log a message about using it). Here are a few other examples of how it should work:

The config can also be specified explicitly:

web-ext --config=/path/to/web-ext-config.json build ...

The config file should apply to global option values as well as sub-command option values. Here is an example of a config that is the equivalent of web-ext --verbose sign --api-url-prefix https://addons-dev.allizom.org/api/v3:

{
  "verbose": true,
  "sign": {
    "api-url-prefix": "https://addons-dev.allizom.org/api/v3"
  }
}

The camel case variants of each option should also be supported, such as:

{
  "sign": {
    "apiUrlPrefix": "https://addons-dev.allizom.org/api/v3"
  }
}

Some additional rules:

  • A command line option will take precedence over any config value
  • An environment variable will take precedence over any config value
  • A config can exist as ~/.web-ext-config.json as well as ./web-ext-config.json (these would override each other in the order listed here).
@kumar303 kumar303 self-assigned this Apr 6, 2016
@kumar303
Copy link
Contributor Author

kumar303 commented Apr 6, 2016

I have a patch started but it depends on yargs/yargs#464

@kumar303
Copy link
Contributor Author

FWIW, a yaml config might be nicer than JSON because you could put comments in it

@MarZab
Copy link

MarZab commented Apr 28, 2016

I would suggest making a .js file that exports a settings object instead.

How about using a grunt task ie. Gruntfile.js like we can with jpm? I feel we are needlessly reinventing the wheel here.

@kumar303
Copy link
Contributor Author

Sure, a .js file would be nice because you can add comments. I think yargs might need a patch to support that but maybe not.

@MarZab
Copy link

MarZab commented Apr 28, 2016

Looks like something like this should work:

var argv = require('yargs')
  .config('settings', function (configPath) {
    return require(configPath)
  }).argv

web-ext.config.js

module.exports = {
  "sign": {
    "apiUrlPrefix": "https://addons-dev.allizom.org/api/v3"
  }
}

@kumar303
Copy link
Contributor Author

oh cool, that looks like it will work. I like using .js config files, let's do it.

@pdehaan
Copy link
Contributor

pdehaan commented Jul 29, 2016

You could use something like rc to use JSON or INI config files.
Or I think ESLint supports config files as:

I believe their specific config loader logic is in eslint/eslint /lib/config/config-file.js.

@kumar303
Copy link
Contributor Author

It looks like rc will parse command line args and env vars but yargs already does that. It would be nice to just fix yargs/yargs#464 but I don't know how difficult that is.

@kumar303
Copy link
Contributor Author

After chatting with @shubheksha yesterday, I think we have a good plan to do this without patching yargs:

  • We will adjust Program.commands() so that it keeps track of any default values
  • We'll write our own config parsing code
    • It will look for ~/.web-ext-config.js and load that file
    • It will look for ./web-ext-config.js and load that, overwriting any previous config values
    • It will look for --config=web-ext-config.js and load that, once again overwriting previous values
    • web-ext will store the JSON config object
    • Config values for a subcommand would need to be named after the command, like: {sign: {apiKey: ...}}
  • Right after we get the yargs config, we will apply any empty values from our config object.
    • This will need to use the cache of default values to figure out if any yargs config value is really empty or not

@rpl
Copy link
Member

rpl commented Jan 2, 2017

@kumar303 @shubheksha I think that it can be useful to keep an eye to how eslint does its config file loading, e.g. some of the relevant pieces:

@kumar303
Copy link
Contributor Author

kumar303 commented Jan 3, 2017

Hi @shubheksha , I split this into three issues (linked above). The patch you've already started applies to #728 so don't worry about any other features that we need later on. This should help land each patch sooner!

@kumar303 kumar303 changed the title Set defaults for all options using a config file Tracker: Set defaults for all options using a config file Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Outreachy
In Progress
Development

No branches or pull requests

6 participants