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

Fix build, Add support for multiple values and empty values #14

Merged
merged 4 commits into from
Aug 14, 2023
Merged

Fix build, Add support for multiple values and empty values #14

merged 4 commits into from
Aug 14, 2023

Conversation

charleywright
Copy link
Collaborator

First of all I want to say thank you for creating this library, I use it in every CLI tool I write due to it's simplicity and robustness. While working on a new project I found I needed to support empty arguments:

./search --file-extension ""

However that causes a crash on line 41 so I fixed that.

I later needed support for multiple values per argument so after looking at #11 I added support for that too. During this process when I tried to build the project I got errors relating to build.bfg so I fixed those to the best of my ability using the examples provided in the bfg9000 repository.

If there are any changes you would like please let me know or if you would prefer separate PR's for each commit. Thanks.

@sailormoon
Copy link
Owner

This is AWESOME! For multiple values, would you be willing to do a template specialization on get for get<vector<T>>? I believe this would allow users to invoke get<vector<string_view>> or get<vector<int32>> etc. without having to change their API invocation to get_multiple.

If you're also willing to, using an unordered_map with a type of vector<optional<string_view>> might be nicer to use internally. The multimap API is a pain to use. I really appreciate your work here.

Let me know your thoughts and thank you again :)

@charleywright
Copy link
Collaborator Author

Switched back to unordered_map as I agree it is a better option. I looked at adding a specialization for get<vector> however since get() returns optional the only suitable specialization would be std::optional<std::vector<...>> which I don't think is what you were looking for. While doing some research on template specializations I came across this question on StackOverflow that would prohibit this specialization from being allowed on a method anyway.

@sailormoon
Copy link
Owner

sailormoon commented Aug 14, 2023

Great investigation -- will squash + merge this and add you as a contributor. Thank you again for finding bugs, adding tests, and expanding the API surface :)

@sailormoon sailormoon merged commit 0321c6f into sailormoon:master Aug 14, 2023
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