-
Notifications
You must be signed in to change notification settings - Fork 5
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
Refactor command error messages #148
Conversation
When a user uses a command with multiple flags wrong, namely `add` and `edit`, the error message only displays an error for the first invalid flag. Now the error message is cumulative, allowing users to correct multiple wrong flag values with 1 error message.
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.
LGTM! Good job on the clean code and on updating all the tests.
|
||
// multiple invalid values but with purely optional fields, only the first in order of logical parsing is | ||
// captured |
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 think this line is still necessary
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.
LGTM! Nice work on implementing this upgrade!
* @param <T> The return type of the method supplied by this supplier. | ||
*/ | ||
@FunctionalInterface | ||
public interface ThrowingSupplier<T> { |
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.
Nice abstraction with ThrowingSupplier!
exclude('**/META-INF/*.SF', '**/META-INF/*.DSA', '**/META-INF/*.RSA') | ||
exclude('**/*.md') // Markdown files | ||
exclude('**/*.txt') // Text files | ||
// exclude('**/*.png', '**/*.jpg') // Images, if not required |
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.
don't forget to clean up this commented lines later!
@@ -26,7 +26,6 @@ public void toModelType_typicalPersonsFile_success() throws Exception { | |||
JsonSerializableAgentAssist.class).get(); | |||
AgentAssist agentAssistFromFile = dataFromFile.toModelType(); | |||
AgentAssist typicalPersonsAgentAssist = TypicalPersons.getTypicalAgentAssist(); | |||
System.out.println("Agent Assist: " + agentAssistFromFile); |
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.
May I ask why?
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 didn't realise you left that in as a debugging comment, let me add that back!
The error message for
add
command is not specific, and does not tell the user which of their flags are missing.In addition, the error messages for add command are too long, requiring the user to horizontally scroll to the right to read it in full.
This can reduce readability.
The proper command messages have been formatted to have more
The error message for
add
command is also able to inform the user which flag is missing.Minor changes:.
Closes #161