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

Implement help command #76

Merged
merged 8 commits into from
Oct 16, 2020

Conversation

JoeyChenSmart
Copy link

@JoeyChenSmart JoeyChenSmart commented Oct 12, 2020

Closes #76

Automatically generate help using the already existing information in the Command's parameters.

Changes to commands

Added a new (implicitly) required constant String for each command, SHORT_DESCRIPTION which will be used in generating help.

We will follow the same convention as commits for these descriptions.

Will use this value when adding new commands to the PrimitiveCommandParser (where you have to add it is obvious when viewing the code).

Future todos?

Will have to add docs to the DG for how this works.

The UI help button still works and shows the old help popup. May have to remove this but we can do this in a separate issue.

So that the PrimitiveCommandParser can focus on parsing. Generating
help is sort of a different responsibility
@codecov-io
Copy link

codecov-io commented Oct 12, 2020

Codecov Report

Merging #76 into master will increase coverage by 0.63%.
The diff coverage is 95.83%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #76      +/-   ##
============================================
+ Coverage     72.58%   73.21%   +0.63%     
- Complexity      415      425      +10     
============================================
  Files            73       73              
  Lines          1375     1400      +25     
  Branches        107      110       +3     
============================================
+ Hits            998     1025      +27     
+ Misses          337      335       -2     
  Partials         40       40              
Impacted Files Coverage Δ Complexity Δ
.../java/jimmy/mcgymmy/logic/commands/AddCommand.java 100.00% <ø> (+13.04%) 4.00 <0.00> (+1.00)
...ava/jimmy/mcgymmy/logic/commands/ClearCommand.java 100.00% <ø> (ø) 2.00 <0.00> (ø)
...va/jimmy/mcgymmy/logic/commands/DeleteCommand.java 100.00% <ø> (ø) 4.00 <0.00> (ø)
...java/jimmy/mcgymmy/logic/commands/EditCommand.java 93.33% <ø> (ø) 5.00 <0.00> (ø)
...java/jimmy/mcgymmy/logic/commands/ExitCommand.java 100.00% <ø> (ø) 2.00 <0.00> (ø)
...java/jimmy/mcgymmy/logic/commands/FindCommand.java 100.00% <ø> (ø) 4.00 <0.00> (ø)
...java/jimmy/mcgymmy/logic/commands/ListCommand.java 100.00% <ø> (ø) 2.00 <0.00> (ø)
.../java/jimmy/mcgymmy/logic/commands/TagCommand.java 100.00% <ø> (ø) 5.00 <0.00> (ø)
...ava/jimmy/mcgymmy/logic/commands/UnTagCommand.java 100.00% <ø> (ø) 5.00 <0.00> (ø)
...mcgymmy/logic/parser/PrimitiveCommandHelpUtil.java 92.00% <92.00%> (ø) 13.00 <13.00> (?)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cdd4de...a59a0e6. Read the comment docs.

@chewypiano chewypiano added this to the v1.3 milestone Oct 13, 2020
@chewypiano chewypiano linked an issue Oct 13, 2020 that may be closed by this pull request
Copy link

@dcchan98 dcchan98 left a comment

Choose a reason for hiding this comment

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

LGTM

@Jh123x Jh123x added the type.Enhancement New feature or request (!DELETE ME IF DASHBOARD IS NOT UPDATING) label Oct 13, 2020
Copy link

@Jh123x Jh123x left a comment

Choose a reason for hiding this comment

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

LGTM!
Works on My Windows Machine too

@JoeyChenSmart
Copy link
Author

JoeyChenSmart commented Oct 13, 2020

Will make this a not draft after I add some tests or something maybe

The code is mostly formatting text to be printed so I dont think unit tests will benefit it much

@Jh123x
Copy link

Jh123x commented Oct 13, 2020

Will make this a not draft after I add some tests or something maybe

The code is mostly formatting text to be printed so I don't think unit tests will benefit it much

Might need to change the logger message in ui/HelpWindow.java and remove the function that shows the help screen

Copy link

@aidoxe-123 aidoxe-123 left a comment

Choose a reason for hiding this comment

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

I think it only needs some minor changes. Other than that, everything LGTM

We don't really need unit tests because help just formats text
@JoeyChenSmart JoeyChenSmart marked this pull request as ready for review October 16, 2020 02:16
@JoeyChenSmart JoeyChenSmart changed the title [WIP] Implement help command Implement help command Oct 16, 2020
Copy link

@Jh123x Jh123x left a comment

Choose a reason for hiding this comment

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

LGTM

@Jh123x Jh123x merged commit 133bfdc into AY2021S1-CS2103T-W17-3:master Oct 16, 2020
@Jh123x Jh123x linked an issue Oct 17, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type.Enhancement New feature or request (!DELETE ME IF DASHBOARD IS NOT UPDATING)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for help method Edit "help" Command to display commands available
6 participants