-
Notifications
You must be signed in to change notification settings - Fork 5
Allow any key in ArgsConfiguration #41
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
base: main
Are you sure you want to change the base?
Conversation
elliVM
left a comment
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.
Looks good thanks!
|
|
||
| if (args.length != 0) { | ||
| final Pattern ptn = Pattern.compile("([A-Za-z.\\-_]+)(=.+)"); | ||
| final Pattern ptn = Pattern.compile("(.+)(=.+)"); |
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.
this allows foo=bar=biz as an assignment where key is foo=bar and value is biz. i find it ambiguous and errorprone. please consider alternative approach.
for example one would foo as key from following and b=ar as a value
foo=b\=ar
but it parses foo=b\ as key and =ar as value
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 non-greedy matching an option? Such as (?<key>.+?)=(?<value>.+) That would result foo being key and b=ar being the value
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.
it is, however it does not do escaping properly, i think.
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.
Not a regex expert but I think I managed to fix it. Now the capture groups allow equal signs only if they are escaped. The regex now expects only one actual equal sign which is between the key and the value. The code then unescapes the equal signs for the user before putting them into the map.
Made some new tests for checking that it is correct.
Fixes #40 .
Previously ArgsConfiguration supported a selected set of characters in the key and any characters in the value of the argument. This had led to many PRs adding more supported characters to the regex as they've been needed.
This PR introduces a regex just checks that the argument is in the "key=value" format (in other words, checks that there is something before and after an equals sign). After evaluating the object, there really is no need to check for illegal characters and the user might have specific needs for the key anyways which is not for the object to restrict.
Added a test for the case where an argument is just the equals sign.