Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Use stdin instead of arguments for wrangler config #239

Merged
merged 8 commits into from
Jul 30, 2019

Conversation

xtuc
Copy link
Member

@xtuc xtuc commented Jun 10, 2019

This changes the way wrangler config works, previously both the email
and the api_key were passed as arguments which would be captured by the
terminal history.

When launching the command it will prompt for an email, once stdin
receive a line it will prompt for the api_key. Both lines are values and
will be stored in the config.

Introduce a WRANGLER_HOME environment variable that will configure
where Wrangler defines its home directory. It will contain a config
directory.

@ashleygwilliams
Copy link
Contributor

i think this is headed in the right direction- how can i help it get across the finish line?

@xtuc
Copy link
Member Author

xtuc commented Jun 14, 2019

I was stuck on the tests, If that looks good to you I can continue.

@ashleygwilliams
Copy link
Contributor

@xtuc sorry for the delayed response- let's definitely finish this up

@ashleygwilliams ashleygwilliams changed the title Config: use stdin instead of arguments [WIP] Config: use stdin instead of arguments Jun 21, 2019
@xtuc
Copy link
Member Author

xtuc commented Jun 25, 2019

Currently the tests are missing, they should open a wrangler config command, enter the data in stdin and test the output (or the config file).

@xtuc xtuc force-pushed the refactor-config-stdin branch from 0a85d50 to ac1280b Compare July 2, 2019 09:06
@xtuc xtuc marked this pull request as ready for review July 2, 2019 09:34
@xtuc xtuc force-pushed the refactor-config-stdin branch 2 times, most recently from c66dcf2 to 9cffd42 Compare July 2, 2019 09:48
@xtuc xtuc changed the title [WIP] Config: use stdin instead of arguments Config: use stdin instead of arguments Jul 2, 2019
@xtuc xtuc force-pushed the refactor-config-stdin branch from 9cffd42 to b313e47 Compare July 2, 2019 11:38
@xtuc xtuc mentioned this pull request Jul 2, 2019
@xtuc xtuc force-pushed the refactor-config-stdin branch from b313e47 to d25d65b Compare July 2, 2019 12:31
@EverlastingBugstopper EverlastingBugstopper changed the title Config: use stdin instead of arguments Config: use stdin instead of arguments for wrangler config Jul 2, 2019
@EverlastingBugstopper EverlastingBugstopper changed the title Config: use stdin instead of arguments for wrangler config Use stdin instead of arguments for wrangler config Jul 2, 2019
@xtuc xtuc force-pushed the refactor-config-stdin branch 7 times, most recently from 20d9e75 to cc7782a Compare July 3, 2019 10:45
@@ -1,5 +1,8 @@
#![allow(clippy::redundant_closure)]

#[macro_use]
extern crate text_io;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: you can get rid of this extern crate + macro use and just do a ‘use text_io::read’

Copy link
Member Author

Choose a reason for hiding this comment

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

text_io only exposes macros and if I use text_io::read! it cannot find macro try_read! in this scope.

tests/config.rs Outdated Show resolved Hide resolved
@ashleygwilliams ashleygwilliams added status - changes requested regression Something is broken, but works in previous releases and removed status - needs review labels Jul 15, 2019
@xtuc xtuc force-pushed the refactor-config-stdin branch from cd52965 to f0f130d Compare July 16, 2019 13:03
This changes the way `wrangler config` works, previously both the email
and the api_key were passed as arguments which would be captured by the
terminal history.

When launching the command it will prompt for an email, once stdin
receive a line it will prompt for the api_key. Both lines are values and
will be stored in the config.

Introduce a `WRANGLER_HOME` environment variable that will configure
where Wrangler defines its home directory. It will contain a `config`
directory.
@xtuc xtuc force-pushed the refactor-config-stdin branch from f0f130d to 0054bf9 Compare July 16, 2019 13:04
xtuc added 2 commits July 16, 2019 14:07
This changes the way `wrangler config` works, previously both the email
and the api_key were passed as arguments which would be captured by the
terminal history.

When launching the command it will prompt for an email, once stdin
receive a line it will prompt for the api_key. Both lines are values and
will be stored in the config.
Avoid the global user config to be system readable, restrict any access
to the current user (600).

Extends the global_config API to allow to specify a configuration
directory to create the configuration file, it's needed for testing.
@@ -42,3 +45,15 @@ fn get_global_config() -> Result<GlobalUser, failure::Error> {
}
}
}

pub fn get_global_config_dir() -> Result<PathBuf, failure::Error> {
let home_dir = if let Ok(value) = env::var("WRANGLER_HOME") {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we (want to) indicate use of this env var in any of our documentation?

Copy link
Member Author

@xtuc xtuc Jul 17, 2019

Choose a reason for hiding this comment

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

Currently it's undocumented, once this lands we can create an issue. Using XDG standarts (#221) might changes how it works.

@ashleygwilliams ashleygwilliams merged commit 9c2ee9d into master Jul 30, 2019
@ashleygwilliams ashleygwilliams deleted the refactor-config-stdin branch July 30, 2019 13:54
Rhilip added a commit to Rhilip/pt-gen-cfworker that referenced this pull request Aug 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
refactor regression Something is broken, but works in previous releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants