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

Optional --config argument for advanced configuration scenarios #501

Merged
merged 3 commits into from
May 17, 2019

Conversation

brandonc
Copy link
Contributor

@brandonc brandonc commented May 16, 2019

Overview

I have an advanced configuration scenario that requires multiple locale targets and with multiple source directories. I want to end up with multiple catalogs that are loaded in different circumstances.

Presently, I work around the obvious config issue by swapping out my .linguirc file in between runs of lingui extract and lingui compile. This isn't ideal and I thought lingui might benefit from a CLI option to specify a configuration file.

This change allows a --config <path> option on add-locale, compile, and extract commands. If the path exists, I use cosmiconfig#loadSync instead of the normal config file probe.

If you find this change welcome, I can work more to add details to the docs.

Tradeoffs

  • Adding a new option could make each command to be perceived as slightly more complex

brandonc and others added 3 commits May 16, 2019 20:06
@codecov
Copy link

codecov bot commented May 17, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@c91de56). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #501   +/-   ##
=========================================
  Coverage          ?   91.13%           
=========================================
  Files             ?       51           
  Lines             ?     1421           
  Branches          ?        0           
=========================================
  Hits              ?     1295           
  Misses            ?      126           
  Partials          ?        0
Impacted Files Coverage Δ
packages/cli/src/lingui-extract.js 69.23% <ø> (ø)
packages/conf/src/index.js 97.14% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c91de56...451f3a6. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented May 17, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@c91de56). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #501   +/-   ##
=========================================
  Coverage          ?   91.13%           
=========================================
  Files             ?       51           
  Lines             ?     1421           
  Branches          ?        0           
=========================================
  Hits              ?     1295           
  Misses            ?      126           
  Partials          ?        0
Impacted Files Coverage Δ
packages/cli/src/lingui-extract.js 69.23% <ø> (ø)
packages/conf/src/index.js 97.14% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c91de56...451f3a6. Read the comment docs.

@tricoder42
Copy link
Contributor

Hey @brandonc, thank you! This is handy! I've added one paragraph to docs and it's ready for release.

Could you please take a look at proposal for new configuration for catalog? I'm gonna finish the PR once I merge #499 and it would be great to get a feedback from you, because you use multiple source directories and multiple locale targets.

@tricoder42
Copy link
Contributor

Released in v2.8.0

@jakubwolny
Copy link

jakubwolny commented May 20, 2019

This is a breaking change for many webpack users who already specify --config option for webpack.

@brandonc
Copy link
Contributor Author

@hamczu I'm having trouble picturing the use case. Are you using webpack-cli to invoke lingui? This should work even if you are chaining commands together (webpack --config myconfig.js && lingui --config myconfig.yaml) Can you provide more details?

@tricoder42
Copy link
Contributor

@brandonc I guess the problem might be regular webpack build - when you run webpack --config ... and you're using @lingui/loader, it'll load incorrect config. We don't check --config presence in command args, but rather in process.args.

@jakubwolny
Copy link

yep, exactly this is happening - I'm just running a webpack dev server: webpack-dev-server --config some-path

and in my (react) application I use @lingui/loader to load the translated messages:
require('@lingui/loader!../en/messages.json')

@brandonc
Copy link
Contributor Author

@tricoder42 It seems to me that for this feature to be compatible with @lingui/loader, there needs to be options loading support added there. I'm working on that change, in addition to passing in command args to getConfig instead of checking process.argv

@brandonc
Copy link
Contributor Author

Fix: #509

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