se-edu / addressbook-level3

:ab::three: Address Book sample application (Level 3)
https://se-education.org/addressbook-level3
MIT License
28 stars 426 forks source link

[#162] Show warning when ignoring repeated parameters #176

Closed Eclipse-Dominator closed 1 year ago

Eclipse-Dominator commented 1 year ago

Fixes #162

Currently, repeated occurrences of the same parameter in a command are being ignored without any notification to the user. This can lead to potential user errors going unnoticed, especially when using the wrong parameter prefix multiple times.

This commit introduces a friendly warning system that alerts the user when a parameter is being ignored. Now, if a parameter is repeated in a command, all occurrences except the last one will trigger a warning message, ensuring that the user is aware of any unintentional mistakes or typos.

Let's provide a better user experience by catching and notifying them of ignored parameters, improving the overall usability and preventing potential errors.

canihasreview[bot] commented 1 year ago

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

canihasreview[bot] commented 1 year ago

v1

@Eclipse-Dominator submitted v1 for review.

(:books: Archive)

Checkout this PR version locally ```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/176/1/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.
codecov-commenter commented 1 year ago

Codecov Report

Merging #176 (ce23ddf) into master (1e05fef) will decrease coverage by 0.23%. The diff coverage is 58.82%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff              @@
##             master     #176      +/-   ##
============================================
- Coverage     72.15%   71.93%   -0.23%     
- Complexity      399      402       +3     
============================================
  Files            70       70              
  Lines          1232     1247      +15     
  Branches        125      131       +6     
============================================
+ Hits            889      897       +8     
- Misses          311      312       +1     
- Partials         32       38       +6     
Impacted Files Coverage Δ
.../seedu/address/logic/parser/EditCommandParser.java 81.25% <33.33%> (-11.06%) :arrow_down:
...java/seedu/address/logic/commands/EditCommand.java 93.42% <72.72%> (-3.60%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

Eclipse-Dominator commented 1 year ago

Hmm, alright, I chose this initially as I felt this will require the least amount of changes but I will come up with a secondary approach to display which field has been duplicated.

damithc commented 1 year ago

Hmm, alright, I chose this initially as I felt this will require the least amount of changes but I will come up with a secondary approach to display which field has been duplicated.

Yup, this may still be the best option but it's better to explore other options before making a decision.

Eclipse-Dominator commented 1 year ago

I have created a secondary PR to show an alternative approach in #181

Eclipse-Dominator commented 1 year ago

Repeated Parameters show warning: #176 Repeated Parameters show warning of repeated prefixes: #181 Repeated Parametes treated as errors: #190

damithc commented 1 year ago

Closing in favor of #190