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

feat: add --clear-cache option #249

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

patricedenis
Copy link
Contributor

This PR is about adding an option to clear the local caches.
It is supposed to delete the files that store the tldr pages locally on your computer.
I was inspired by the same option that already exists on the tldr-node-client.

It may resolve #242 has after application, no local cache should remain on the storage.
Also I'm not sure that the workflow automates the build of the man page (this is not modified in current PR).
If not, I could try to modify the building workflow to allow that.

I would again be very pleased if someone could test this PR locally before merging if wanted because I haven't set a development environment yet.

@kbdharun kbdharun self-assigned this Jun 28, 2024
@kbdharun kbdharun changed the title Feature add : clear cache option feat: add --clear-cache option Jun 28, 2024
Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I have some suggestions for this PR.

tldr.py Outdated Show resolved Hide resolved
- Added iteration over multiple languages, handling non-English languages dynamically.
- Implemented recursive directory traversal to delete files and subdirectories within cache directories.
- Improved error handling with detailed messages for file and directory deletion failures.
thanks to @kbdharun

Co-authored-by: K.B.Dharun Krishna <[email protected]>
@kbdharun kbdharun closed this Jun 28, 2024
@kbdharun kbdharun reopened this Jun 28, 2024
@kbdharun kbdharun added this to the 3.4.0 milestone Jun 29, 2024
@kbdharun kbdharun requested a review from vitorhcl July 11, 2024 05:46
Copy link
Member

@vitorhcl vitorhcl left a comment

Choose a reason for hiding this comment

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

We can simplify the function using shutil (I can't add a suggestion to add the needed import)

Comment on lines +501 to +519
# Recursively walk through the directory to delete files and subdirectories
for root, dirs, files in os.walk(cache_dir, topdown=False):
for name in files:
file_path = Path(root) / name
try:
file_path.unlink() # Attempt to delete the file
except Exception as e:
print(f"Error: Unable to delete cache file {file_path}: {e}")
for name in dirs:
dir_path = Path(root) / name
try:
dir_path.rmdir() # Attempt to delete the directory
except Exception as e:
print(
f"Error: Unable to delete cache directory {dir_path}: {e}"
)
# Attempt to remove the main cache directory after clearing its contents
try:
cache_dir.rmdir()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Recursively walk through the directory to delete files and subdirectories
for root, dirs, files in os.walk(cache_dir, topdown=False):
for name in files:
file_path = Path(root) / name
try:
file_path.unlink() # Attempt to delete the file
except Exception as e:
print(f"Error: Unable to delete cache file {file_path}: {e}")
for name in dirs:
dir_path = Path(root) / name
try:
dir_path.rmdir() # Attempt to delete the directory
except Exception as e:
print(
f"Error: Unable to delete cache directory {dir_path}: {e}"
)
# Attempt to remove the main cache directory after clearing its contents
try:
cache_dir.rmdir()
try:
shutil.rmtree(cache_dir)

@kbdharun
Copy link
Member

kbdharun commented Oct 4, 2024

Hi @patricedenis, any updates on this?

@patricedenis
Copy link
Contributor Author

Hi. I haven't been involved recently, I'm sorry. I will try to look for the sugested shutil addition.

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.

tldr --update_cache does not delete local-only pages
3 participants