Skip to content
This repository was archived by the owner on Jan 10, 2025. It is now read-only.

[Touch.Server] Fix parsing of -address, -port and -logpath options#97

Merged
spouliot merged 3 commits intoxamarin:mainfrom
rpendleton:fix-option-parsing
Jan 8, 2021
Merged

[Touch.Server] Fix parsing of -address, -port and -logpath options#97
spouliot merged 3 commits intoxamarin:mainfrom
rpendleton:fix-option-parsing

Conversation

@rpendleton
Copy link
Contributor

Because the ip, port and logpath options were missing = at the end of their names, you couldn't actually pass any values for them. If you tried, the string values for the options would be equal to their names (rather than what the user provided), and then the parsing failures for those strings would be silently ignored.

Additionally, after the IPAddress was successfully parsed, we ignored the result. This resulted in listener.Address being null, which eventually led to a crash once the listener was started. I fixed this as well.

In addition to adding the = to each option, I also changed the parsing from TryParse to Parse so that invalid option values will fail with an exception. This is similar to how the TimeSpan options are already being handled, but I wasn't sure if it would be considered a breaking change. I just figured it's better to let the user know something is wrong vs silently ignore invalid input from them, but I can revert that change if necessary.

@rpendleton
Copy link
Contributor Author

rpendleton commented Jan 8, 2021

This PR is similar to #22 which was closed without comment, but I'm assuming it was just closed because of the whitespace noise. It's definitely still an issue though, and this fixes #21.

@rpendleton rpendleton changed the title [Touch.Server] Fix parsing of -address, -port and -logfile options [Touch.Server] Fix parsing of -address, -port and -logpath options Jan 8, 2021
Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

minor: style only

Co-authored-by: Sebastien Pouliot <sebastien.pouliot@gmail.com>
@spouliot spouliot merged commit 425c3b2 into xamarin:main Jan 8, 2021
@rpendleton rpendleton deleted the fix-option-parsing branch October 9, 2021 05:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants