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

Parse configuration file from current directory for additional flags #197

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

aloussase
Copy link
Contributor

This is tackling #160

I added a separate record Flags to hold the flags that can be parsed from a configuration file, tentatively dotenv.config in the current directory.

At first, I tried to reuse the Options record, but this became messy when dealing with the program argument. I could not find a non-hacky way to deal with that and thought that it would be best to have a separate parser for the flags.

Having said that, I extracted the option parsers to separate definitions to reuse as much code as possible and also so that the parsers for flags and options keep in sync.

applyFlags takes care of combining the flags from command line arguments and the configuration file. <> for list arguments is a natural combination, and I believe || for booleans also works correctly.

Some execution examples:

# dotenv.config
-x .env.example
-f .env
# .env.example
API_URL=
# .env

Command:

dotenv echo hello

Output:

dotenv: The following variables are present in .env.example, but not set in the current environment, or .env: API_URL
CallStack (from HasCallStack):
  error, called at src/Configuration/Dotenv.hs:82:16 in dotenv-0.12.0.0-inplace:Configuration.Dotenv

Another example:

# dotenv.config
echo hello

Command:

dotenv echo hello

Output:

There were errors while parsing the configuration file
Invalid argument `echo'

Usage: dotenv [-v|--version] [-f|--dotenv DOTENV] [-x|--example DOTENV_EXAMPLE] 
              [-o|--overload] [--verbose] [--dry-run] [-D|--no-dups]

  Runs PROGRAM after loading options from FILE

I print a special message when errors come from the config file so the user knows where to look.

I might be missing something, what do you think?

Also, I had to add directory as a dependency to check for the file existence, but if there is a better way to do this without adding an extra dependency that would be great.

Copy link
Collaborator

@CristhianMotoche CristhianMotoche left a comment

Choose a reason for hiding this comment

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

@aloussase Thanks a lot for your contribution! It looks great. Thank you so much for putting effort on this. I really like it! I was wondering if you can take a look at the issue in the CI since that's stopping us from merging this PR.

@aloussase
Copy link
Contributor Author

Hi @CristhianMotoche , fixed the CI issue. Let me know if you need anything else :)

Copy link
Collaborator

@CristhianMotoche CristhianMotoche left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @aloussase

@CristhianMotoche CristhianMotoche merged commit 030fbda into stackbuilders:main Oct 18, 2024
7 checks passed
@CristhianMotoche
Copy link
Collaborator

@all-contributors please add @aloussase for code

Copy link
Contributor

@CristhianMotoche

@aloussase already contributed before to code

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