jannaleong / pe

0 stars 0 forks source link

a/ prefix for create command is case senstive #5

Open jannaleong opened 4 months ago

jannaleong commented 4 months ago

As per UG, i have used the create command. However, i have capitalised the prefix A Input : ic/S0123456C hp/87654321 A/311, Clementi Ave 2, #02-25 dob/1990-01-01 s/F st/PENDING e/janed@example.com bt/A+ n/Cane Doe Output : invalid command format error

image.png

I expected the command to be accepted as prefixes should be case-insensitive. However, this comomand was blcoked. This behaviour is problematic because it is not optimised for fast typist that could easilt make a mistkae.

soc-pe-bot commented 4 months ago

Team's Response

Both issues are about prefix not being case sensitive, which are related to the same mechanism in argument tokenizer and setting of prefixes.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

/hp prefix is not case sensitive for the create command

As per UG, i have used the create command. However, i have capitalised the prefix HP Input : ic/S0123456C HP/87654321 a/311, Clementi Ave 2, #02-25 dob/1990-01-01 s/F st/PENDING e/janed@example.com bt/A+ n/Cane Doe Output : invalid command format error

image.png

I expected the command to be accepted as prefixes should be case-insensitive. However, this comomand was blcoked. This behaviour is problematic because it is not optimised for fast typist that could easilt make a mistkae.


[original: nus-cs2103-AY2324S2/pe-interim#3014] [original labels: severity.Low type.FeatureFlaw]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

Thank you for pointing this out. It is a useful enhancement for fast typers. Nonetheless, all the prefixes were implemented to take in lowercase letters only for consistency among functions, and the app in its current form works as such.

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: [replace this with your explanation]


## :question: Issue response Team chose [`response.NotInScope`] - [x] I disagree **Reason for disagreement:** As this is a duplicated issue, i have attached the same explanation as the original bug report: (it is the same explanation but instead of hp, it should be a) As your group has pointed out, making hp prefix case insensitive is a " useful enhancement for fast typers". As this tp project is one that explicitly caters to fast typists and that should be optimised for fast typists, I do not believe that this is an enhancement that is beyond the scope of this project and can be considered as not in scope. It is a fundamental requirement that the you include enhancements to make the lives of fast typists easier when they use your app. Furthermore, it is also mentioned in the tp bug guidelines that an example of a bug is "making keywords case-sensitive when there is no need for it". This can be seen below. There is no reason for your prefixes, which is a keyword that is used frequently, to be case sensitive as you do not have another prefix with exactly the same letters. Furthermore, the fix for this requires minimal effort as you simply have to include a .lower() to your input string. With this simple fix, you allow your users the flexibility to enter their commands even if they have typos that accidentally capitalise your prefix. Especially as you are trying to cater to fast typists, such typos can occur frequently as they are typing very quickly. Thus this case-insensitivity of "a" prefix is a valid aspect of your command that you have forgotten to consider. Thus, as it is not optimised enough for fast typists, it is a feature flaw. ![image.png](https://raw.githubusercontent.com/jannaleong/pe/main/files/b89b5629-e0b1-404f-b883-8f1678bc17ea.png)