Skip to content
This repository has been archived by the owner on Jan 28, 2020. It is now read-only.

Feat commander #158

Open
wants to merge 66 commits into
base: develop
Choose a base branch
from
Open

Feat commander #158

wants to merge 66 commits into from

Conversation

apachano
Copy link
Contributor

Adds a command registry and CLI interface.
The CommandBook is the registry that stores commands for use by a Commander. A Commander is constructed with a CommandBook and two streams (one in and one out) that can either be cin/cout for terminal use with the server or point to streams in the client (the chat terminal). The commander contains a loop that checks for input and executes commands.

The commanders have 3 built-in functions that don't exist in the CommandBook, these are hardcoded and can't be overwritten. A help function, a list function, and an exit function.

@apachano apachano requested a review from a team as a code owner October 27, 2019 04:54
Copy link
Member

@vyomfadia vyomfadia left a comment

Choose a reason for hiding this comment

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

Run a Clang format before the final merge, but the code seems good, just a few nitpicks I have. Get it compiling though! 😛

Quartz/Engine/Include/Quartz/Utilities/Commander.hpp Outdated Show resolved Hide resolved
Quartz/Engine/Include/Quartz/Utilities/Commander.hpp Outdated Show resolved Hide resolved
Quartz/Engine/Source/Utilities/Commander.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@DontBelieveMe DontBelieveMe left a comment

Choose a reason for hiding this comment

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

In general, the code looks good :D but like beeper says if you could run clang-format on the changes, and address the few comments I've left - only little things so nothing major.

Overall though, good job 👍

Quartz/Engine/Include/Quartz/Utilities/Commander.hpp Outdated Show resolved Hide resolved
Quartz/Engine/Include/Quartz/Utilities/Commander.hpp Outdated Show resolved Hide resolved
Quartz/Engine/Include/Quartz/Utilities/Commander.hpp Outdated Show resolved Hide resolved
Quartz/Engine/Source/Utilities/Commander.cpp Outdated Show resolved Hide resolved
Quartz/Engine/Source/Utilities/Commander.cpp Outdated Show resolved Hide resolved
Quartz/Engine/Include/Quartz/Utilities/Commander.hpp Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@apachano apachano mentioned this pull request Dec 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants