Zer0Legion
commented
5 months ago Team's Response
Definitions: Before our explanation, we would like to define some key terms we will use: For an example of “add … s|S12 r|R12”
- “|”: We will refer to this as the pipe symbol.
- “r|, s|, etc.”: We will refer to these as prefixes.
- “S12, R12”: We will refer to the portion after the prefixes as arguments
Explanation: We understand that from the user perspective this might seem like the application is showing an incorrect error. However this is intended behaviour as we would want to treat the extra input as part of the studio’s argument.
Our application is designed to ONLY split the user input using the case-sensitive prefixes. This is so that if the previous prefix has an argument that includes the pipe symbol in their input, the application will still register it as part of the previous argument, rather than treat it as an incorrect prefix. This is a preemptive design choice that we maintained to ensure that future prefixes can use the pipe symbol as part of their argument rather than throw an error just because the pipe symbol is used. .
For Example: If we implemented a remarks function and wished to use “|” as part of a remark and we typed add ….. remark| Remedial Class R| Lecture 8 to add a remark of “Remedial Class R| Lecture 8”, the app would now incorrectly throw an error when we identify the R| as part of the user input string.
Considering that our application has plans to make our features more general and expand our use cases to courses beyond CS1101S, we decided that this trade off was one that we should make to handle the issue of the pipe symbol being used in arguments.
Our DG has elaborated on the parsing of user input under the “Logic” section, which would have directed any potential developer in the direction of ArgumentTokenizer if they had queries with regards to prefix parsing.
Under the UG, we have also failed to explicitly mention that prefixes are case sensitive. However, we have included a prefix table which includes every prefix that can be used, with all of the prefixes being lowercase. As such, users would very much be aware that the prefixes used should be in lowercase. We have also included examples for users to reference and modify should they encounter an error in using the application that they cannot fix and identify.
As such, we propose that this report be shifted to the domain of documentation bugs, as we are missing documentation information in both the UG and DG. We also believe that this flaw is only deserving of a “Low” tag, as once again, any misunderstandings in usage on both the developer and user side can be resolved by viewing the section on prefix parsing in the DG or looking at the examples and parameter usage provided in the UG respectively.
Additionally due to the above reasons, we also propose to change the type of this bug report to NotInScope.
Duplicate status (if any):
--
Description: Wanted to add this code "add n|John Doe p|98765432 e|johsdsdsdnd@example.com a|John street, block 123, #01-01 m|A1234567Z s|Ssdsdsd1 r|R2t|"
Although there is no space between r|R2t|, the UG didn't stated that there must be spaces between prefixes
Steps to reproduce: Enter this code add n|John Doe p|98765432 e|johsdsdsdnd@example.com a|John street, block 123, #01-01 m|A1234567Z s|Ssdsdsd1 r|R2t|
Expected: new student should be added
Actual: Error message: Reflections in the style of R1.. are accepted. The first character must be "R", followed by up to 4 digits.
Screenshots:
[original: nus-cs2103-AY2324S2/pe-interim#3474] [original labels: type.FunctionalityBug severity.Low]