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

Refactor filesystem with pathlib + improve CLI UX #19

Closed
wants to merge 3 commits into from
Closed

Refactor filesystem with pathlib + improve CLI UX #19

wants to merge 3 commits into from

Conversation

sderev
Copy link
Contributor

@sderev sderev commented Jun 15, 2023

08be4aa

This commit refactors the way we interact with the filesystem by replacing the os.path module with pathlib.Path. This makes the code more readable, easier to understand and less error-prone.

Key changes include:

  • Introduced a global variable, LOG_DB, to represent the path of the log database file. This eliminates the need for the get_log_db_path() function.
  • Replaced the os.path.exists checks with the Path.exists() method.
  • Replaced the os.path.expanduser calls and string concatenations for file paths with the / operator provided by pathlib.Path.
  • Used the Path.mkdir() method for directory creation.
  • Improved the formatting of string messages by using f-strings for more readability and flexibility in the future.
  • A helpful instruction has been added to the exception raised when no log database is found, guiding the user to run llm init-db, similar to the scenario when ~/.llm/log.db is absent and a chat history needs to be read.

These changes should improve the maintainability and readability of the code without altering the functionality.


9cb5224

This commit improves the CLI UX by adding the help text for no-arg scenarios; however, it preserves the pipe compatibility—the prompt can still be manipulated with --system.

Running llm without any arguments no longer causes the program to hang indefinitely.


974f9d2

Add a helper message for the logs command.

This will be displayed in the general --helper message.

This commit refactors the way we interact with the filesystem by replacing the `os.path` module with `pathlib.Path`. This makes the code more readable, easier to understand and less error-prone.

Key changes include:

* Introduced a global variable, `LOG_DB`, to represent the path of the log database file. This eliminates the need for the `get_log_db_path()` function and aligns.
* Replaced the `os.path.exists` checks with the `Path.exists()` method.
* Replaced the `os.path.expanduser` calls and string concatenations for file paths with the `/` operator provided by `pathlib.Path`.
* Used the `Path.mkdir()` method for directory creation.
* Improved the formatting of string messages by using f-strings for more readability and flexibility in the future.
* A helpful instruction has been added to the exception raised when no log database is found, guiding the user to run `llm init-db`, similar to the scenario when `~/.llm/log.db` is absent and a chat history needs to be read.

These changes should improve the maintainability and readability of the code without altering the functionality.
@sderev sderev changed the title Refactor filesystem with pathlib Refactor filesystem with pathlib + improve CLI UX for no-arg scenarios Jun 15, 2023
Add help text for no-arg scenarios, but preserve pipe compatibility.
This will be displayed in the general `--helper` message.
@sderev sderev changed the title Refactor filesystem with pathlib + improve CLI UX for no-arg scenarios Refactor filesystem with pathlib + improve CLI UX Jun 15, 2023
@simonw
Copy link
Owner

simonw commented Jun 15, 2023

I'm actively rewriting this code at the moment so I won't be able to merge this. Thanks for the suggestions!

@simonw simonw closed this Jun 15, 2023
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.

2 participants