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 CompleteCommand to only accept either Index or LocalDate #206

Merged

Conversation

Kb-Tay
Copy link
Collaborator

@Kb-Tay Kb-Tay commented Nov 5, 2023

Previously, there was a bug that allowed both arguments to be input together

Updated the parser to only allow either one of the arguments but not both at the same time, else the command usage message will be rendered to user.

This is to make it clearer to prevent confusing behaviour for the user

Resolves #199

Previously, there was a bug that allowed both arguments to be input together

Updated the parser to only allow either one of the arguments but not both at the same time, else the command usage message will be rendered to user.

This is to make it clearer to prevent confusing behaviour for the user
@Kb-Tay Kb-Tay added bug Something isn't working documentation Improvements or additions to documentation labels Nov 5, 2023
@Kb-Tay Kb-Tay added this to the v1.4 milestone Nov 5, 2023
Copy link

codecov bot commented Nov 5, 2023

Codecov Report

Merging #206 (10f6d71) into master (4f59dfd) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #206      +/-   ##
============================================
- Coverage     75.67%   75.66%   -0.02%     
- Complexity      666      667       +1     
============================================
  Files           101      101              
  Lines          2154     2153       -1     
  Branches        227      228       +1     
============================================
- Hits           1630     1629       -1     
  Misses          461      461              
  Partials         63       63              
Files Coverage Δ
.../seedu/address/logic/commands/CompleteCommand.java 84.44% <100.00%> (-0.98%) ⬇️
...du/address/logic/parser/CompleteCommandParser.java 100.00% <100.00%> (ø)

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@sopa301
Copy link
Collaborator

sopa301 commented Nov 5, 2023

I think it would be good to update the UG to specify the exact behaviour as well, since I'm not too sure exactly which inputs are accepted and which are not.

Copy link
Collaborator

@sopa301 sopa301 left a comment

Choose a reason for hiding this comment

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

LGTM

@sopa301 sopa301 merged commit f1833f0 into AY2324S1-CS2103T-F12-1:master Nov 5, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PE-D][Tester A] complete command confusing arguments and results
2 participants