tingkai-mai / pe

0 stars 0 forks source link

Misleading `rfind`/`sfind` Format #12

Open tingkai-mai opened 2 years ago

tingkai-mai commented 2 years ago

Issue:

image.png

The main description/Command Summary's rfind shows that t/[KEYWORD] is not optional, when in fact it is. If it was truly optional, it would be displayed as [t/KEYWORD], with the bracket wrapped around the whole parameter. This is misleading, as now when I try to find by rfind n/someName t/, nothing appears.

I would say this is High severity, because if I try to execute the rfind command strictly, and execute t/, then I would also find reviews with no tags, which might not be what I want. I might want to see all reviews with that specified name, regardless of whether or not it has tags.

This is the same for the sfind command.

To Reproduce:

N/A

Expected Output:

N/A

Actual Output:

N/A

nus-pe-script commented 1 year ago

Team's Response

The main description/Command Summary's rfind shows that t/[KEYWORD] is not optional, when in fact it is. If it was truly optional, it would be displayed as [t/KEYWORD], with the bracket wrapped around the whole parameter.

Documentation is not accurate that the t/ keyword is optional.

This is misleading, as now when I try to find by rfind n/someName t/, nothing appears.

Nothing appears as there are no reviews with names someName. eg. rfind n/Alex t/ on the sample data returns a review.

I would say this is High severity, because if I try to execute the rfind command strictly, and execute t/, then I would also find reviews with no tags, which might not be what I want. I might want to see all reviews with that specified name, regardless of whether or not it has tags.

This is still a usable feature even if the user does not know that t/ can be omitted and should not warrant the high severity.

This is the same for the sfind command.

Items for the Tester to Verify

:question: Issue severity

Team chose [severity.Low] Originally [severity.High]

Reason for disagreement: [replace this with your explanation]