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

feat(command-env): add support for plaintext output in env:list #5322

Merged
merged 3 commits into from
Dec 21, 2022

Conversation

tinfoil-knight
Copy link
Contributor

@tinfoil-knight tinfoil-knight commented Dec 17, 2022

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes #3262

The env:list command currently outputs an ASCII table by default which isn't helpful if someone intends to write the environment variables to a file.

The --plain flag introduced in this PR would allow a user to setup their env files easily on local systems since it outputs the environment variables in the plaintext format used in .env files.

Example Output here:

$ netlify env:list --plain
NODE_ENV=development
REACT_APP_API_URL=https://api.netlify.com
REACT_APP_SECRET=UILH0lQphn

Also see: #3262 (comment)


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)
🦥

@tinfoil-knight tinfoil-knight requested a review from a team as a code owner December 17, 2022 07:51
@tinfoil-knight tinfoil-knight added type: feature code contributing to the implementation of a feature and/or user facing functionality area: command: env labels Dec 17, 2022
@github-actions
Copy link

github-actions bot commented Dec 17, 2022

📊 Benchmark results

Comparing with 0bc18bb

Package size: 249 MB

(no change)

^  249 MB  249 MB  249 MB  249 MB  249 MB  249 MB  249 MB  249 MB  249 MB  249 MB  249 MB  249 MB  249 MB 
│   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

Copy link
Contributor

@jasonbarry jasonbarry left a comment

Choose a reason for hiding this comment

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

Amazing, thanks so much for pushing this along @tinfoil-knight!! 🙌

I have a request that shouldn't be too much lift: considering that we also have the undocumented --json flag, and that --json --plain doesn't make much sense, could this change to --format <format>?

  • --format would have an array of choices of table, plain, and json, with table being the default
  • options.json should still be supported on line 71, alongside options.format === 'json', to preserve backwards-compatibility

Let me know what you think. Thanks again for making this contribution! 💯

@@ -135,6 +135,7 @@ netlify env:list
**Flags**

- `context` (*string*) - Specify a deploy context or branch (contexts: "production", "deploy-preview", "branch-deploy", "dev")
- `plain` (*boolean*) - Output environment variables as plaintext
Copy link
Contributor

Choose a reason for hiding this comment

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

Could new entries to the help be put below scope? Would be nice to keep context and scope next to each other. Thanks!

Copy link
Contributor Author

@tinfoil-knight tinfoil-knight Dec 20, 2022

Choose a reason for hiding this comment

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

The plain flag in between seemed weird to me too but the options are sorted here to produced the documentation:

export const sortOptions = (optionA, optionB) => {
// base flags should be always at the bottom
if (BASE_FLAGS.has(optionA.long) || BASE_FLAGS.has(optionB.long)) {
return -1
}
return optionA.long.localeCompare(optionB.long)
}

Although, we can rename the option to text so it appear after scope.


Other alternatives like raw, plaintext would appear in between context & scope in the sorted order.

Let me know if you've a better suggestion or if we should go ahead with text.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha -- let's keep it as plain, and let the docs sort them as they currently do. But in --help, it would be great if context and scope could appear next to each other.

Copy link
Contributor Author

@tinfoil-knight tinfoil-knight Dec 20, 2022

Choose a reason for hiding this comment

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

Unfortunately, it seems like we're using the same function to order the help output too so I don't think the re-ordering would be possible unless we rename the option.

The current order does appear a bit weird to me too but I don't think there's any way to override this specific instance.

Ref:

const optionList = helper
.visibleOptions(command)
.sort(sortOptions)
.map((option) => formatItem(helper.optionTerm(option), helper.optionDescription(option)))
if (optionList.length !== 0) {
output = [...output, chalk.bold('OPTIONS'), formatHelpList(optionList), '']
}
}

@tinfoil-knight
Copy link
Contributor Author

tinfoil-knight commented Dec 20, 2022

I have a request that shouldn't be too much lift: considering that we also have the undocumented --json flag, and that --json --plain doesn't make much sense, could this change to --format <format>?

  • --format would have an array of choices of table, plain, and json, with table being the default
  • options.json should still be supported on line 71, alongside options.format === 'json', to preserve backwards-compatibility

@jasonbarry I'm slightly opposed to making the suggested changes.

  • The default flow first prints the table with env values hidden (key, scope visible) & then asks if the user wants to print the actual env values or not. The --json & --plain flags just print the env key & values directly which makes their behaviour different than the default one. I don't think values passed to the --format flag should have different behaviour (apart from the output format).

  • The --json flag is used across a lot of commands across the CLI so users familiar with the CLI would expect the JSON output for relevant commands to be accessible through the same flag. Adding the new --format flag to env:list would make this experience inconsistent with rest of the CLI commands.

WDYT?

P.S. I found a way to avoid the conflict with --json --plain.

The commander CLI framework supports adding a conflicts property to the options to prevent conflicting options from being used together.

$ ncli env:list --json --plain
 ›   Error: option '--plain' cannot be used with option '--json'
 ›   See more help with --help

@jasonbarry
Copy link
Contributor

Sounds like a plan @tinfoil-knight, thanks for the research and explanation. I'd be happy to approve with this proposal if a conflicts property is added.

@tinfoil-knight
Copy link
Contributor Author

@jasonbarry The conflicts property has been added now to prevent the --json & --plain flags from being used together.

Copy link
Contributor

@jasonbarry jasonbarry left a comment

Choose a reason for hiding this comment

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

Awesome work @tinfoil-knight, thanks for adding this feature!

@tinfoil-knight tinfoil-knight added the automerge Add to Kodiak auto merge queue label Dec 21, 2022
@kodiakhq kodiakhq bot merged commit 12664f3 into netlify:main Dec 21, 2022
@tinfoil-knight tinfoil-knight deleted the env-list-plain branch December 21, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: command: env automerge Add to Kodiak auto merge queue type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add function like env:export to make .env files from site config
2 participants