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

Update parser #87

Merged

Conversation

mingyang143
Copy link

@mingyang143 mingyang143 commented Oct 21, 2024

fixes #88
fixes #72
Changed:

  • prefix in parser to make it easier for users to type commands
  • hours is optional when adding a tutee or tutor

@mingyang143 mingyang143 added this to the v1.4 milestone Oct 21, 2024
@mingyang143 mingyang143 requested a review from a team October 21, 2024 09:20
@mingyang143 mingyang143 self-assigned this Oct 21, 2024
public static final Prefix PREFIX_TAG = new Prefix("t/"); // TODO change?
public static final Prefix PREFIX_SUBJECT = new Prefix("-subject ");
public static final Prefix PREFIX_FILEPATH = new Prefix("-filepath ");
public static final Prefix PREFIX_NAME = new Prefix("\\n ");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remember to enable the disabled tests!

Copy link

@kohkakohla kohkakohla left a comment

Choose a reason for hiding this comment

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

LGTM in general, but do fix the GFMD.

argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_HOURS);
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get());
Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get());
Email email = ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get());
Hours hours = ParserUtil.parseHours(argMultimap.getValue(PREFIX_HOURS).get());
Address address = ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get());
Set<Tag> tagList = ParserUtil.parseTags(argMultimap.getAllValues(PREFIX_TAG));
Set<Subject> subjects = ParserUtil.parseSubjects(argMultimap.getAllValues(PREFIX_SUBJECT));

Choose a reason for hiding this comment

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

Nice! Removed dead code. Well done!

|| !argMultimap.getPreamble().isEmpty()) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddTutorCommand.MESSAGE_USAGE));
}

// Hours is optional for adding a tutee and will be set to 0 if unspecified

Choose a reason for hiding this comment

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

Good comments letting the reader know what the code does. Very readable, very demure!


#### On macOS:
```shell
Cmd + Space

Choose a reason for hiding this comment

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

Hey Mingyang, I don't think command + space is the default shortcut for opening the terminal. Maybe you might want to look into this further?

Copy link

@kohkakohla kohkakohla left a comment

Choose a reason for hiding this comment

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

LGTM! Well done, Mingyang! Excellent work as always. Cheers!

@kohkakohla kohkakohla merged commit ed7b6ae into AY2425S1-CS2103T-F08-1a:master Oct 21, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants