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

Branch fix ug #233

Merged

Conversation

QinHaichen12
Copy link

Don't merge yet, changes are pending

@QinHaichen12 QinHaichen12 added priority.Medium Nice to have type.UserGuide A change related to the userguide labels Nov 11, 2024
@QinHaichen12 QinHaichen12 added this to the v1.6 milestone Nov 11, 2024
@QinHaichen12 QinHaichen12 self-assigned this Nov 11, 2024
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ Complexity Δ
...ter/address/logic/commands/FindSessionCommand.java 86.66% <100.00%> (-2.23%) 6.00 <0.00> (-1.00)

@QinHaichen12
Copy link
Author

Find Session now always displays "Matching sessions found" and does not include the session names

- Words in `UPPER_CASE` are the parameters to be supplied by the user.<br>
e.g., in `add_member n/NAME`, `NAME` is a parameter that can be used as `add_member n/John Doe`.
- Items in square brackets are optional.<br>
- Words in `UPPER_CASE` are the placeholders you'll replace with your own input.<br>

Choose a reason for hiding this comment

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

Fantastic wording, that helps a lot.

Why is UPPER_CASE like this instead of UPPER CASE though, or do you think it doesn't matter either way?

Copy link
Author

Choose a reason for hiding this comment

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

I think in the words formatted as such underscore is used to separate words, assumed that its a convention

- Items in square brackets are optional.<br>
- Words in `UPPER_CASE` are the placeholders you'll replace with your own input.<br>
e.g., in `add_member n/NAME`, `NAME` can be replaced with `John Doe ` The full command will be `add_member n/John Doe`.
- Fields in `[square brackets]` are optional. You can choose to include them or leave them out.<br>

Choose a reason for hiding this comment

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

Are we using "fields", "parameters" or sth else going forwards?

Copy link
Author

Choose a reason for hiding this comment

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

fields are used in the e.g. statements so i changed to that

e.g., `n/NAME [tag/TAG]` can be used as `n/John Doe tag/friend` or as `n/John Doe`.
- Items with `…`​ after them can be used multiple times, including zero times.<br>
e.g., `[tag/TAG]…​` can be used as ` ` (i.e., 0 times), `tag/friend`, `tag/friend tag/family`, etc.
- Fields with `…`​ after them can be used multiple times or not at all.<br>

Choose a reason for hiding this comment

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

technically, we always use ... in conjunction with [] so this addition is incorrect.
practically, I think this way is indeed easier to read.

wdyt?

Copy link
Author

Choose a reason for hiding this comment

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

in that case should we change delete_session from delete_session s/NAME m/INDEX… to delete_session s/NAME m/INDEX [m/INDEX]…?

Choose a reason for hiding this comment

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

YES can't believe I forgot

@taggyhan
Copy link

lgtm

@starchypotatocode starchypotatocode merged commit 8173435 into AY2425S1-CS2103T-W14-3:master Nov 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority.Medium Nice to have type.UserGuide A change related to the userguide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants