You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Claudenw (via GitHub)" <gi...@apache.org> on 2024/01/08 12:44:23 UTC
[PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Claudenw opened a new pull request, #217:
URL: https://github.com/apache/commons-cli/pull/217
Fix for CLI-322
Adds kabab-format options and extensive option name testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on PR #217:
URL: https://github.com/apache/commons-cli/pull/217#issuecomment-1912201204
@garydgregory updated as requested
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #217:
URL: https://github.com/apache/commons-cli/pull/217#issuecomment-1880987515
@Claudenw
I think you mean "kabab" -> "kebab". Always run `mvn` before you push with the latest from git master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #217:
URL: https://github.com/apache/commons-cli/pull/217#issuecomment-1886795176
## [Codecov](https://app.codecov.io/gh/apache/commons-cli/pull/217?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`c1f473c`)](https://app.codecov.io/gh/apache/commons-cli/commit/c1f473c5ff7c270f4db5f567d344a2ed0d8533ba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 93.20% compared to head [(`ab004a0`)](https://app.codecov.io/gh/apache/commons-cli/pull/217?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 93.22%.
> Report is 4 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #217 +/- ##
============================================
+ Coverage 93.20% 93.22% +0.02%
- Complexity 570 575 +5
============================================
Files 21 21
Lines 1207 1211 +4
Branches 215 217 +2
============================================
+ Hits 1125 1129 +4
Misses 46 46
Partials 36 36
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/commons-cli/pull/217?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CLI-322: Add '-' as an option char and implemented extensive tests [commons-cli]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #217:
URL: https://github.com/apache/commons-cli/pull/217
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #217:
URL: https://github.com/apache/commons-cli/pull/217#issuecomment-1925738269
Hi @Claudenw
Run `mvn` locally before you push to catch these build failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on PR #217:
URL: https://github.com/apache/commons-cli/pull/217#issuecomment-1892002190
@garydgregory Looks like it works. Can I get a review plz.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on PR #217:
URL: https://github.com/apache/commons-cli/pull/217#issuecomment-1886778417
> @Claudenw I think you mean "kabab" -> "kebab". Always run `mvn` before you push with the latest from git master.
Indeed, I do not know how to spell. :(
I did run 'mvn' before the push and everything worked. Got "BUILD SUCCESS" result. Just tried again and same thing. I am addressing the issues from the build now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on PR #217:
URL: https://github.com/apache/commons-cli/pull/217#issuecomment-1886800937
The build fails on 22-ea which fails with a spotbugs execution error, It seems like spotbugs is not yet supported on 22-ea.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #217:
URL: https://github.com/apache/commons-cli/pull/217#issuecomment-1887806937
RE 22-ea, that should be fixed by the next commons-parent release, so don't worry about that part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Added '-' as an option char and implemented extensive tests [commons-cli]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #217:
URL: https://github.com/apache/commons-cli/pull/217#discussion_r1452357445
##########
src/main/java/org/apache/commons/cli/OptionValidator.java:
##########
@@ -23,14 +23,35 @@ Licensed to the Apache Software Foundation (ASF) under one or more
* @since 1.1
*/
final class OptionValidator {
+ /* package private for testing access */
+ /** The array of additional characters allowed as the first character in the option but not in the rest of the option */
+ static final char[] ADDITIONAL_OPTION_CHARS = {'?', '@'};
+ /** The array of additional characters allowed in the rest of the option but not in the first position */
+ static final char[] ADDITIONAL_LONG_CHARS = {'-'};
+
+ /**
+ * Checks the char array for a matching char.
+ * @param ary the array to search
+ * @param c the char to look for.
+ * @return {@code true} if {@code c} was in {@code ary}, {@code false} otherwise.
+ */
+ private static boolean search(final char[] ary, final char c) {
Review Comment:
'ary' is a poor parameter name here, almost any else else is better: 'chars', 'array, 'buffer'...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org