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

Pages that are only in common, always force an https connection #118

Open
masoudd opened this issue May 20, 2020 · 3 comments
Open

Pages that are only in common, always force an https connection #118

masoudd opened this issue May 20, 2020 · 3 comments
Milestone

Comments

@masoudd
Copy link

masoudd commented May 20, 2020

common vs specific platform

Take for example the command: tldr tldr on linux (Let's assume I already have the file common/tldr.md in my cache directory and it is newer than TLDR_CACHE_MAX_AGE).

One might reasonably assume that no https requests will be made. But in fact, the program first looks for linux/tldr.md, since it's not in the cache, it first requests it from
PAGES_SOURCE_LOCATION/linux/tldr.md
which returns a 404. then after all this, it looks for common/tldr.md and shows the contents.

Solution

I suggest checking the common cache directory before making an https connection for platform specific files. Or to avoid changing default behavior, at least put this behavior behind an option like
--common-before-net

@zlatanvasovic
Copy link
Contributor

zlatanvasovic commented May 20, 2020

https://github.com/tldr-pages/tldr/blob/master/CLIENT-SPECIFICATION.md says nothing about it, but we will take a look. Your reasoning makes perfect sense.

@zlatanvasovic
Copy link
Contributor

@MasterOdin Wanna take a look at this?

@MasterOdin
Copy link
Collaborator

I would probably just make it the default that we construct the list of possible page locations, check if the page exists in any of them, and if it does, display it (assuming page is not too old), else start making http calls. I plan to work on this on a broader clean-up of the process to determine this information (as well as start adding some unit tests to the library).

@MasterOdin MasterOdin added this to the 1.1.0 milestone Aug 1, 2020
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

No branches or pull requests

3 participants