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

Auto update cache by default? #250

Closed
uhthomas opened this issue Jan 3, 2022 · 8 comments · Fixed by #254
Closed

Auto update cache by default? #250

uhthomas opened this issue Jan 3, 2022 · 8 comments · Fixed by #254
Labels
Milestone

Comments

@uhthomas
Copy link

uhthomas commented Jan 3, 2022

Hi!

Thanks for writing tealdeer, it's been great to use.

I've found having to manually update the cache to be confusing, especially on first-use. Consider the reference tldr package versus tealdeer:

❯ paru -S tldr
❯ tldr tldr

  Displays simple help pages for command-line tools, from the tldr-pages project.
  More information: <https://tldr.sh>.
❯ paru -S tealdeer
❯ tldr tldr
Warning: Cache not found. Please run `tldr --update`.

I had to dig throug the documentation and GitHub issues to find #90 and https://dbrgn.github.io/tealdeer/config_updates.html. At which point I had to seed the config:

❯ tldr --seed-config

Then enable auto cache update:

[updates]
-auto_update = false
+auto_update = true
auto_update_interval_hours = 720

Does it make sense to enable this behaviour by default?

@dbrgn
Copy link
Collaborator

dbrgn commented Jan 3, 2022

Right now I'd prefer not to enable auto-updates by default, but we should improve the documentation with regards to cache management.

@dbrgn dbrgn added the docs label Jan 3, 2022
@niklasmohrin
Copy link
Collaborator

niklasmohrin commented Jan 3, 2022

I am also against enabling auto-update by default.

If it is not against spec, we could inform about the existence of auto_update in the result text of tldr --update

@dbrgn
Copy link
Collaborator

dbrgn commented Jan 3, 2022

That sounds like a good idea as well. And/or link to the docs.

@uhthomas
Copy link
Author

uhthomas commented Jan 3, 2022

I see. Would you be able to help me understand why enabling auto-updated by default may not be desirable? Is there some sort of middleground to be reached where the first run of tealdeer fills the cache? More simply; can tealdeer fill the cache if empty instead of forcing the user to run tldr -u? What are your thoughts?

Thank you for the quick responses by the way! 😄

@niklasmohrin
Copy link
Collaborator

I think the main point is that we don't want to go ahead and start placing files on the system right after installing the program / without users explicitly telling us to do so. Personally, I see the client as an offline-only tool with the occasional updating of the cache

@dbrgn
Copy link
Collaborator

dbrgn commented Jan 5, 2022

Yes, I see it like @niklasmohrin. Downloading files and placing them in your file system should be done with user consent by default. It's the same with most other index/cache based tools on Linux, for example package managers like apt or pacman. However, we can definitely highlight the auto-update feature more prominently.

@uhthomas
Copy link
Author

uhthomas commented Jan 5, 2022

Your points make sense and I agree that auto-update by itself may not be a desirable default.

As suggested in my previous comment, is there some sort of middleground to be met where tldr -u is not a requirement to use tealdeer? At current, tealdeer won't work without first running tldr -u. This is not the case with the reference implementation where it just works. This can be observed with containers:

❯ docker run docker.io/archlinux sh -c 'pacman -Sy --noconfirm tldr && tldr tldr'
...
installing tldr...
:: Running post-transaction hooks...
(1/1) Arming ConditionNeedsUpdate...

  tldr

  Displays simple help pages for command-line tools, from the tldr-pages project.
  More information: https://tldr.sh.

  - Show the tldr page for a command (hint: this is how you got here!):
    tldr command

  - Show the tldr page for `cd`, overriding the default platform:
    tldr -p android|linux|osx|sunos|windows cd

  - Show the tldr page for a subcommand:
    tldr git-checkout

  - Update local pages (if the client supports caching):
    tldr -u

Whereas tealdeer:

❯ docker run docker.io/archlinux sh -c 'pacman -Sy --noconfirm tealdeer && tldr tldr'
...
installing tealdeer...
:: Running post-transaction hooks...
(1/1) Arming ConditionNeedsUpdate...
Cache not found. Please run `tldr --update`.

@niklasmohrin
Copy link
Collaborator

While there is an increase in ease of use, falling back to looking up pages has several downsides for us.

  1. More code to maintain for page lookup itself
  2. Configuration - sometimes, you might really not want to go online (say, you are on a tethered connection), so configuration options must be added
  3. Added benchmark setups. We have setup benchmarks against other clients and we would have to I would expect us to benchmark both on- and offline lookup
  4. Right now, you can "trust" tealdeer to never connect to the internet if you didn't explicitly allow it to do so. In my opinion, this behavior is a virtue and we should always try to keep it that way

Furthermore, I don't think that having to run tldr --update once is a problem at all, we are telling users exactly what they need to do to get started with tealdeer. Probably, they are already in the "setting-things-up" mode as they installed the software seconds ago.

I agree that we could promote auto-update more directly, but for the other thing, I don't really think that the benefits outweigh the costs here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants