Skip to content
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

Fixed reading from file when using standard separator (" ") #222

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TKlerx
Copy link
Contributor

@TKlerx TKlerx commented Mar 13, 2015

When specifying a file with parameters and " " as separator, JCommander crashed because the parameters and their values were not split correctly.
This is the (ugly) fix.

@TKlerx TKlerx force-pushed the master branch 2 times, most recently from 2f76521 to f225dda Compare April 8, 2015 13:29
pull recent changes from cbeust repo
@TKlerx
Copy link
Contributor Author

TKlerx commented Apr 23, 2015

Any comment on accepting or rejecting this pull request?

@jeremysolarz
Copy link
Contributor

@TKlerx I know it's been a while but any way to provide a test case for it?

@mkarg
Copy link
Collaborator

mkarg commented Oct 3, 2023

@TKlerx Thank you for your contribution. As @jeremysolarz already said, it would be good having a test case. I need to confess that I do not understand the actual problem just from reading the bug description. Thank you.

Copy link
Collaborator

@mkarg mkarg left a comment

Choose a reason for hiding this comment

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

Please add test case. Thanks.

@@ -38,6 +38,8 @@
import java.util.Map;
import java.util.ResourceBundle;

import bsh.classpath.BshClassPath.GeneratedClassSource;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You are adding an import but you are not referencing it.

@@ -481,9 +500,9 @@ private static List<String> readFile(String fileName) {

try {
BufferedReader bufRead = new BufferedReader(new FileReader(fileName));

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove whitespace added at the end of the line.

List<String> splitFileArgs = Lists.newArrayList();
//special treatment for standard separator (" ")
String[] v1 = fileArgs.toArray(new String[0]);
for (int i = 0; i < fileArgs.size(); i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please correct indentation.

List<String> splitFileArgs = Lists.newArrayList();
//special treatment for standard separator (" ")
String[] v1 = fileArgs.toArray(new String[0]);
for (int i = 0; i < fileArgs.size(); i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why introducing i instead of simply using for-each?

if (isOption(v1, arg2)) {
String sep = getSeparatorFor(v1, arg2);
if (" ".equals(sep)) {
String[] sp = arg2.split("[" + sep + "]", 2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why using sep here instead of static " "?

for (String ssp : sp) {
splitFileArgs.add(ssp);
}
}else{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please add blanks around else?

@mkarg
Copy link
Collaborator

mkarg commented Jul 6, 2024

@TKlerx Kindly requesting your response! :-)

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.

3 participants