Skip to content

Add support for optimistic cache functionality, separate cli and config parsing from run #125

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kahlstrm
Copy link

@kahlstrm kahlstrm commented Apr 6, 2025

First of all, thanks for this wonderful piece of software. I Have been enjoying it for over a year!

However, one particular caveat I have encountered is when I run tldr after a
a couple of weeks to remind me of e.g. how to use tar (that already
exists in the cache), I first have to wait to download a cache update prior
I want to see the result already in my local (but stale) cache.

Alternatively, I could use --offline, but then I would need to run tldr --update manually sometimes,
I would need to run the command twice for new commands, first with --offline and then without.

This PR tries to address this by adding a --optimistic-cache-flag that, when enabled, adds the following cases:

  • If the cache was found but is stale, it does the lookup first against the stale cache and prints out the result if it is found.
    After displaying the result, it runs the update and also communicates to the user if the cache is out of date.
  • If the entry is not in the stale cache, we will re-try the lookup by updating the stale cache first.

To avoid doing massive amounts of refactoring, I had to do a bit of hack by first
separating CLI and config parsing from the run-function, and then in the optimistic cache
the case where the page is not found, call run recursively by disabling the optimistic cache flag.

Let me know if

  • This feature is unwanted, and I should use --offline instead.
  • The proposed solution is too hacky, and you want me to refactor/do more code-splitting for the PR instead.

@acuteenvy
Copy link
Member

Alternatively, I could use --offline, but then I would need to run tldr --update manually sometimes

If that delay annoys you, then you can always set up something to run tldr --update for you automatically.
Here's an example using systemd:

[Unit]
Description=Update the cache of tldr pages
After=network.target

[Service]
Type=oneshot
ExecStart=tldr --update

[Install]
WantedBy=default.target

Put that in ~/.config/systemd/user/tldr-update.service and run systemctl --user enable tldr-update. The cache will be updated on every boot when you log in as your user, in the background. You can then set auto_update = false in the tlrc config, since it doesn't make sense to have automatic updates before viewing a page with this configuration.


As for this PR, yeah the recursive run() hack is something I'd like to avoid, but that can be refactored quite easily. I'm more concerned about whether this feature is useful. The delay we're talking about is a couple seconds, every two weeks (and that can be changed already if you'd like to update less often). On the off chance that this does irk someone, you can always run updates in the background.

Let's get some more opinions though, I'll wait for other maintainers.

@kahlstrm
Copy link
Author

kahlstrm commented Apr 7, 2025

Alternatively, I could use --offline, but then I would need to run tldr --update manually sometimes

If that delay annoys you, then you can always set up something to run tldr --update for you automatically. Here's an example using systemd:

[Unit]
Description=Update the cache of tldr pages
After=network.target

[Service]
Type=oneshot
ExecStart=tldr --update

[Install]
WantedBy=default.target

Put that in ~/.config/systemd/user/tldr-update.service and run systemctl --user enable tldr-update. The cache will be updated on every boot when you log in as your user, in the background. You can then set auto_update = false in the tlrc config, since it doesn't make sense to have automatic updates before viewing a page with this configuration.

As for this PR, yeah the recursive run() hack is something I'd like to avoid, but that can be refactored quite easily. I'm more concerned about whether this feature is useful. The delay we're talking about is a couple seconds, every two weeks (and that can be changed already if you'd like to update less often). On the off chance that this does irk someone, you can always run updates in the background.

Let's get some more opinions though, I'll wait for other maintainers.

Yeah, found out about tldr-update-service in Home Manager right after filing this PR, and that seems to do similar things as that systemd-configuration. The only problem for me is that it currently does not support macOS that I use at work. But yeah, I guess I could write a manual implementation of that systemd service for macOS as well, and that would do it 👍.

Nevertheless, if this is deemed worthwhile, I can do the work to remove the hacky recursiveness, but I'll wait for confirmation before doing that if this is a wanted feature or not.

@devnoname120
Copy link

@acuteenvy Hmm for macOS and Windows it would be a different syntax, not ideal when managing one's dotfiles. I also find it a bit of an annoyance. IMO the cache update should happen after the command (if stale), not before because it means having to wait a bit which can interrupt the flow. It's not a huge thing, just a little thing that I think would be nice.

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.

3 participants