Skip to content

Conversation

@sallerga
Copy link
Contributor

@sallerga sallerga commented Dec 4, 2023

#541

Do not call enumerator.Current if enumerator.MoveNext has not been called or returns false.

public bool MoveNext()
{
if (Current == null || !Current.MultipleValues)
if (!_currentValid || !Current.MultipleValues)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Current is not nullable so I removed the null check. I think it was only used to check if MoveNext had been called. Not to allow Current to be null while iterating. Please let me know if I should add it again.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's also better because it not only checks that MoveNext has been called, but also that it returned true. The error was caused by calling Current after MoveNext has returned false.

Copy link
Contributor

@thomaslevesque thomaslevesque left a comment

Choose a reason for hiding this comment

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

LGTM. Better than my own attempt, actually, so I'll close my PR.

public bool MoveNext()
{
if (Current == null || !Current.MultipleValues)
if (!_currentValid || !Current.MultipleValues)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's also better because it not only checks that MoveNext has been called, but also that it returned true. The error was caused by calling Current after MoveNext has returned false.

@Gav-Brown
Copy link

Just to say thanks. Looking forward to this fix.

@prodigy
Copy link

prodigy commented Jan 10, 2024

Thank you for already fixing this, I'd have submitted a PR now otherwise. 🙏

@connorstorer-kbxcom
Copy link

FYI: I've got a work-around that's working for my scenario.

#541 (comment)

@paul-gauci
Copy link

I really need this fix :( . Can i do something to help?

@prodigy
Copy link

prodigy commented Feb 6, 2024

I really need this fix :( . Can i do something to help?

Just fork the repo, fix it there, and include the source in your solution.

And try this workaround:
#541 (comment)

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.

.NET 8: CommandLineApplication.Execute with unknown command throws System.InvalidOperationException: Enumeration already finished

7 participants