feat: add TLDR_PLATFORM env variable to override platform detection #292
Conversation
|
Be patient. People will review them when they have time. |
Thanks ! |
README.md
Outdated
| - `linux` | ||
| - `windows` | ||
| - `win32` | ||
| - `android` | ||
| - `darwin` | ||
| - `freebsd` | ||
| - `macos` | ||
| - `netbsd` | ||
| - `openbsd` | ||
| - `osx` | ||
| - `sunos` | ||
|
|
There was a problem hiding this comment.
I believe some of these don't exist.
Add support for the TLDR_PLATFORM environment variable to let users specify the desired platform (e.g., "linux", "macos", "windows") for fetching TLDR pages. This overrides the default platform detection. The --platform option remains supported for per-command overrides. Signed-off-by: Itamar Shalev <[email protected]>
Signed-off-by: Itamar Shalev <[email protected]>
ItamarShalev
force-pushed
the
tldr_platform_env
branch
from
fe1d796 to
5f5e76c
Compare
Signed-off-by: Itamar Shalev <[email protected]>
ItamarShalev
force-pushed
the
tldr_platform_env
branch
from
5f5e76c to
080137c
Compare
tldr.py
Outdated
| @@ -59,6 +59,8 @@ | |||
| "windows": "windows" | |||
| } | |||
|
|
|||
| SUPPORTED_PLATFORMS = sorted(set(OS_DIRECTORIES.values())) | |||
There was a problem hiding this comment.
I think this should be using the keys (the platform name), as the values are the actual directories. 👀 Otherwise you won't be able to specify darwin, or win32 👍🏻
| SUPPORTED_PLATFORMS = sorted(set(OS_DIRECTORIES.values())) | |
| SUPPORTED_PLATFORMS = sorted(set(OS_DIRECTORIES.keys())) |
There was a problem hiding this comment.
That's the idea
As I asked here: #292 (comment)
I have no problem to allow the symlinks
There was a problem hiding this comment.
Although they don't actually exist in tldr and aren't symlinks, I think it makes sense to support the aliases to be honest as that's the already supported values for --platform on this client (and the Node client).
Or, I guess we can keep it as-is with .values() to stay in-line with --platform on the C client.
I will leave this for someone with more experience in client dev to decide. 👍🏻
There was a problem hiding this comment.
We can support it but don't specify this option in the readme.md, agreed ?
There was a problem hiding this comment.
We can probably just say to look at the --platform flag for available platforms, rather than repeating them.
Signed-off-by: Itamar Shalev <[email protected]>
Signed-off-by: Itamar Shalev <[email protected]>
Signed-off-by: Itamar Shalev <[email protected]>
Signed-off-by: Itamar Shalev <[email protected]>
Signed-off-by: Dharun Krishna K B <[email protected]>
Signed-off-by: Dharun Krishna K B <[email protected]>
kbdharun
changed the title
|
This PR caused #306 |
Add support for the TLDR_PLATFORM environment variable to let users specify the desired platform (e.g., "linux", "macos", "windows") for fetching TLDR pages.
This overrides the default platform detection.
The --platform option remains supported for per-command overrides.