-
Notifications
You must be signed in to change notification settings - Fork 11
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 various bugs #119
Fix various bugs #119
Conversation
// covers the "-lruntime=debug,parachain=trace" case | ||
// TODO: Make this more generic by adding the scenario in the regex below | ||
if (v.starts_with("-l") || v.starts_with("-log")) && v.contains(',') { | ||
return Ok(Arg::Flag(v.to_string())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically we are set this as flag instead of traing to parse it right? Did you try if there is no "," like -lparachain=debug ??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. This should be better parsed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is ok to treated as flag, but I think maybe there is a bug if isn't contains a ","
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the case of not containing "," and indeed it breaks; As a result I removed that ||
and retested the possible scenarions and now in both cases a Flag
(as needed) returns
Coverage after merging nik-toml-example into main
Coverage Report
|
Based on the simple example, various bugs have been identified;
This PR resolves them