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

Work on multiline input #12

Merged
merged 2 commits into from
Nov 1, 2023
Merged

Work on multiline input #12

merged 2 commits into from
Nov 1, 2023

Conversation

ChaoticRoman
Copy link
Owner

Some attempt to address this.

Closes #2

@github-actions
Copy link

The code changes look good overall. Here are some points for consideration:

  1. It's great that you have added argparse to handle command-line arguments. However, the description in argparse.ArgumentParser(description='') is empty. It would be helpful to provide a brief description of what the script does.

  2. The function cli_input_multiline() could use a docstring to explain its purpose and how it works, especially the part about typing "SEND" to finish the input.

  3. In the function cli_input_multiline(), you are checking for the exit commands after joining all lines of user input. If a user decides to exit while still inputting, they would have to finish their input before being able to exit. You might want to consider checking for exit commands inside the while loop.

  4. In the main() function, you could add a docstring to explain what the function does.

  5. It's good that you have added a main guard (if __name__ == "__main__":). This allows the script to be imported as a module in another script, and the main function will only run if the script is run directly.

  6. Make sure to update any relevant documentation to reflect the new multiline mode and how to use it.

  7. Lastly, ensure that the new code does not break any existing tests. If there are no tests, I would recommend adding some to make sure the new features work as expected.

Copy link

github-actions bot commented Nov 1, 2023

Prompt too long for OpenAI: 4054 characters, sending only first 4000 characters
The code changes look good and are well-structured. Here are some suggestions and points to consider:

  1. Comments and Documentation: The code is well-commented and the classes have docstrings which is great. It improves readability and understanding of the code.

  2. Code Structure: The code is well-structured and modular. The separation of different functionalities into different functions like cli_input, cli_input_multiline, check_exit etc. is good.

  3. Error Handling: There is no explicit error handling in the code. Consider adding try-except blocks to handle potential errors. For example, in the cli_input_multiline function, there might be an error if the user interrupts the input.

  4. Test File: It's great that you have added a test file. However, it seems the test file is not complete as the last print statement is incomplete. Also, consider using a testing framework like pytest or unittest for more structured testing.

  5. Argument Parsing: The argument parsing in the main function is well-implemented. The help descriptions for the arguments are clear and concise.

  6. Code Duplication: The check_exit function is called in both cli_input and cli_input_multiline functions. This is a bit repetitive and could be improved. Consider calling check_exit once in the main function.

  7. Commit Message: The commit message is not included in the provided information. It's always good practice to provide meaningful commit messages that give a clear idea of what changes have been made in the commit.

  8. PEP8 Compliance: The code seems to follow PEP8 guidelines for Python code style. This includes naming conventions, line length, and whitespace usage.

Overall, the code changes are well-implemented and follow good coding practices. The suggestions above can help improve the code further.

@ChaoticRoman ChaoticRoman merged commit 39183b8 into main Nov 1, 2023
1 check passed
@ChaoticRoman ChaoticRoman deleted the multiline-cli branch September 12, 2024 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cli is not handling multiline input
1 participant