Skip to content

update command line argument parser#19

Merged
mhshami01 merged 2 commits intomainfrom
user/hshami/verbose_is_boolean
May 17, 2021
Merged

update command line argument parser#19
mhshami01 merged 2 commits intomainfrom
user/hshami/verbose_is_boolean

Conversation

@mhshami01
Copy link
Contributor

  • modified the add_arg function to take multiple arguments.
  • enabled boolean type options.
  • added a 'reset' for argument parser.
  • update parser's unit tests

# compare output, should be empty
if [ "${parsed_cmds[*]}" != "" ];
then
echo "Failed to pass unit test 'test_illegal_flags' in test-cmd-parser.sh. Non-empty result: ${parsed_cmds[*]}"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a good place to use the loggers for our unit test outputs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know. Let's discuss this point during planning...

done

for arg in "$@"
while [ $# -ne 0 ];
Copy link
Contributor

Choose a reason for hiding this comment

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

for my own learning, is it better to use a while instead of a for loop in these kind of situations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1- Since I need to look at $1 and $2, the $arg and $2 seemed inconsistent.
2- I personally find this form simpler and less error prone.

if [[ "$valid_arg" == "false" ]];
if [[ "$valid_argument" == "false" ]];
then
parsed_cmd=()
Copy link
Contributor

Choose a reason for hiding this comment

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

Let user know when the arguments are invalid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to leave this for a later iteration. I think it is a large enough scope to require its own PBI.

@marianan
Copy link
Contributor

VERSION_TAG="v0.0.0-rc0"

Could you please increment the version and release after you merge your changes? I had this in my PR but didn't get to complete the PR on Friday


Refers to: src/utils.sh:6 in 0a350d9. [](commit_id = 0a350d9, deletion_comment = False)

Copy link
Contributor

@marianan marianan left a comment

Choose a reason for hiding this comment

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

:shipit:

@mhshami01 mhshami01 merged commit f56d95b into main May 17, 2021
@mhshami01 mhshami01 deleted the user/hshami/verbose_is_boolean branch May 17, 2021 17:47
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.

3 participants