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

[WIP] Enviroment var config #380

Merged
merged 6 commits into from
Nov 4, 2018
Merged

Conversation

deg4uss3r
Copy link
Contributor

Work in progress PR for #375

Happy to change as you see fit, tested manually with a --style=numbers line, looking for a good way to write an integration test for the config $VAR

src/config.rs Outdated Show resolved Hide resolved
@sharkdp
Copy link
Owner

sharkdp commented Oct 26, 2018

Looks good, thank you very much for your contribution!

@sharkdp
Copy link
Owner

sharkdp commented Oct 26, 2018

If you want to write an integration test for this (which would be great), you could take a look at the pager_basic test in integration_tests.rs for an example on how to set an environment variable.

You would probably have to add an exemplary config file somewhere in the tests folder and set the BAT_CONFIG_PATH variable to that file.

@sharkdp
Copy link
Owner

sharkdp commented Nov 4, 2018

Thank you very much for the updates. I made some small modifications on top of yours.

@deg4uss3r
Copy link
Contributor Author

Cool, these make sense! Is there anything else you would like me to add? I can type up an addition to the README if you’d like.

@sharkdp
Copy link
Owner

sharkdp commented Nov 4, 2018

I can type up an addition to the README if you’d like.

Oh, that would be great! Unfortunately, there is no section about the config file, yet (see #377).

@deg4uss3r
Copy link
Contributor Author

see #377

Sure! I will open a new PR for that. First commit will be very basic to discuss how it should be formatted and getting all the options included. Is that okay?

@sharkdp sharkdp merged commit 278d841 into sharkdp:master Nov 4, 2018
@sharkdp
Copy link
Owner

sharkdp commented Nov 4, 2018

Sure! I will open a new PR for that. First commit will be very basic to discuss how it should be formatted and getting all the options included. Is that okay?

Sounds great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants