-
Notifications
You must be signed in to change notification settings - Fork 4
feat: implement server functionality #64
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
Conversation
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com> Signed-off-by: P4rth-B <parth_b1@cs.iitr.ac.in>
…ient handling Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
…ization Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
…ing and directory operations Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
…paths and improve logging Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
…le_request Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
…play in TUI Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
…tory paths in get_file_info and list_directory Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements core server functionality and updates the client‐server connection handling while improving logging configuration and test coverage. Key changes include refactoring the client handler interface (from ClientHandler to IClientHandler), updating connection helper functions to return a status flag along with ClientInfo, and integrating enhanced logging in server/client modules.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unittests/server/server_connection_manager_test.cpp | Updated mock client handler inheritance and modified the test client connection helper to return a status flag. |
| src/server/server.cpp, connection_manager.cpp | Added and improved logging, adjusted client handling logic and non‐blocking mode configuration. |
| src/server/main.cpp, src/client/main.cpp | Revised logging initialization and command-line argument parsing. |
| include/server/, include/common/ | Updated interfaces and documentation to reflect the new client_handler abstraction and logging enhancements. |
| src/common/file_operations.cpp | Revised file modified time retrieval using std::chrono conversion. |
Comments suppressed due to low confidence (2)
include/server/connection_manager.hpp:88
- Ensure that all references to the client handler use the updated interface (IClientHandler) consistently across the codebase and update any related comments/documentation to avoid confusion with the deprecated ClientHandler.
void set_client_handler(std::unique_ptr<IClientHandler> handler);
src/client/client.cpp:149
- [nitpick] Verify that updating the current directory using response.data() consistently reflects the intended server response format for 'cd' commands, and consider renaming variables if necessary for clarity.
m_tui->update_current_directory(response.data());
| } | ||
|
|
||
| ClientInfo connect_test_client() | ||
| std::pair<ClientInfo, bool> connect_test_client() |
Copilot
AI
May 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the documentation/comments for 'connect_test_client' to reflect that it now returns a pair containing both ClientInfo and a boolean flag indicating connection success.
DarkPhoenix42
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
closes #61
closes #62