Skip to content

feat(asc): Add options merge algorithm for use by asconfig #1343

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

Merged
merged 8 commits into from
Jun 17, 2020

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Jun 16, 2020

This is an attempt to solve the reconciliation problems encountered during #1238 by extending the existing options parser with a backtracking merge algorithm.

With this, we'd essentially start by parsing command line arguments, obtaining an initial set of options, then merge the project's asconfig.json if applicable and continue by merging parent project asconfig.jsons until we reach the end. Once done, we can add option defaults that we so far omitted.

The options enable and disable now have an additional mutuallyExclusive key indicating that whatever is newer in the respective other equalizes what's older. Does not affect the same level, but only merging a previous level.

There is a basic test in tests/cli/options.js, containing the following full usage test:

var result = optionsUtil.parse(["--enable", "a", "--disable", "b"], config, false);
merged = optionsUtil.merge(config, result.options, { enable: ["b", "c"] });
merged = optionsUtil.merge(config, merged, { disable: ["a", "d"] });
optionsUtil.addDefaults(config, merged);
assert.deepEqual(merged.enable, ["a", "c"]);
assert.deepEqual(merged.disable, ["b", "d"]);
assert.deepEqual(merged.other, ["x"]);

@jtenner Does this look like it'd help / work in all scenarios?

  • I've read the contributing guidelines

@dcodeIO dcodeIO changed the title Add options merge algorithm for use by asconfig refactor(asc): Add options merge algorithm for use by asconfig Jun 16, 2020
@dcodeIO dcodeIO changed the title refactor(asc): Add options merge algorithm for use by asconfig feat(asc): Add options merge algorithm for use by asconfig Jun 16, 2020
Copy link
Contributor

@jtenner jtenner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow this does the job. And it looks way more professional. Does this try to resolve asconfig files yet?

@MaxGraey
Copy link
Member

MaxGraey commented Jun 16, 2020

Should we have also array of booleans?

    case "B": {
      if (!Array.isArray(value)) value = [ value ];
      return value.map(Boolean);
    }

@dcodeIO
Copy link
Member Author

dcodeIO commented Jun 16, 2020

We don't have these in other code paths, and I suspect that these won't be useful. At least I can't imagine an option where a [true, true, false, true ] would make a lot of sense.

@dcodeIO dcodeIO merged commit 3163abb into master Jun 17, 2020
@github-actions
Copy link

🎉 This PR is included in version 0.12.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@dcodeIO dcodeIO deleted the asconfig-merge branch July 16, 2020 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants