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

Add test case for short option handling #1260

Merged
merged 3 commits into from Apr 24, 2022
Merged

Conversation

dearchap
Copy link
Contributor

@dearchap dearchap commented Mar 28, 2021

…erminated error

What type of PR is this?

  • bug

What this PR does / why we need it:

The short options handling needs to restart from failed processing rather than from beginning of arguments

Which issue(s) this PR fixes:

Fixes #1254

Special notes for your reviewer:

Testing

go test -run=TestSliceShortOptionHandle ./...

Release Notes


@dearchap dearchap requested a review from a team as a code owner Mar 28, 2021
@dearchap dearchap requested review from rliebz and coilysiren and removed request for a team Mar 28, 2021
@coilysiren coilysiren requested review from a team and removed request for rliebz and coilysiren Apr 23, 2021
@dearchap dearchap changed the title Fix: (issue#1254) short options handling needs to proceed from last t… Fix: (issue#1254) Add test case for short option handling Apr 29, 2021
@stale
Copy link

stale bot commented Jul 29, 2021

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

@stale stale bot added the status/stale stale due to the age of it's last update label Jul 29, 2021
@stale
Copy link

stale bot commented Aug 28, 2021

Closing this as it has become stale.

@stale stale bot closed this Aug 28, 2021
@dearchap
Copy link
Contributor Author

dearchap commented Aug 28, 2021

@rliebz rliebz reopened this Aug 30, 2021
@stale
Copy link

stale bot commented Aug 30, 2021

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

@stale stale bot removed the status/stale stale due to the age of it's last update label Aug 30, 2021
@dearchap
Copy link
Contributor Author

dearchap commented Dec 25, 2021

Ping. Simple change and its just in test code . @coilysiren @rliebz

Copy link
Member

@rliebz rliebz left a comment

Apologies for the delay, and thanks for the contribution!

Just wanted to double check, issue #1254 was already fixed in some other change set, and the purpose for this PR is to add a regression test?

flag_test.go Outdated Show resolved Hide resolved
flag_test.go Outdated Show resolved Hide resolved
flag_test.go Outdated Show resolved Hide resolved
flag_test.go Show resolved Hide resolved
@meatballhat meatballhat changed the title Fix: (issue#1254) Add test case for short option handling Add test case for short option handling Apr 21, 2022
@meatballhat meatballhat added kind/bug describes or fixes a bug area/v2 relates to / is being considered for v2 labels Apr 21, 2022
@meatballhat meatballhat added this to the Release 2.5.0 milestone Apr 21, 2022
@meatballhat meatballhat changed the base branch from master to main Apr 21, 2022
Copy link
Member

@meatballhat meatballhat left a comment

🎉 Thank you, @dearchap!

@meatballhat meatballhat merged commit c864c24 into urfave:main Apr 24, 2022
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StringSlice() with UseShortOptionHandling causes duplicated entries, depending on the ordering of the flags
3 participants