-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Define spec for officially sanctioned clients #1065
Comments
We should close #1010 and #1000 and make it part of this issue. Mentioning a comment made by @rubenvereecken for reference -
The main idea was to enforce a |
Are you saying that every sanctioned client must implement auto-completion? If not (which is my position), I'd say we should connect those issues to this one, but not close them. That way we can track implementation of that feature without blocking this one. A |
Not at all. But I was saying if they are to be implemented at all, it should be implemented by clients(because a proper auto-completion should also complete the helper flags and not just commands, and the parent repo has no way to know all the helper flags of all the clients). That's what we concluded at the end of the PR. Therefore, I want a |
Ok, in that case those issues should remain open until (1) we define what a sanctioned client is --i.e. this issue--, (2) we identify which clients are (or we can easily help become) sanctioned, and finally (3) we ensure that the sanctioned clients all support auto-completion. And this issue will make sure that |
Sure. |
Writing down some notes that I collected. Architecture
I think nothing else should be specified regarding how the client wishes to store the pages internally. If it wishes to store the metadata in a RocksDB cache, that should be allowed. Features
This should be a good starting point. |
Good points. Some observations:
|
According to tldr-pages/tldr-node-client#52, we'll also want the clients to support a |
Another feature we should probably also include: auto-updating the pages content. Quoting from #1056 (comment):
|
Super helpful notes, deserving of a more prominent place! Ray Lee's client fetches single pages that aren't cached yet or which have expired, but I haven't seen another bash client that uses the zip file. I think requiring a json config file is not a nice thing to implement in a bash client. Great if your language has easy libraries for that... Adding autocomplete for bash is possible, but more intrusive for the installation. It is nice to just need 1 file in the right location, and you're good to go. |
What do you think about #876?
In that case we would need to include that in the client spec. I don't see why it wouldn't work, but I'd prefer if other client authors commented here.
Related previous discussions: #336, #343, #718. I think we should shorten the archive formats we officially provide for clients as much as possible, to encourage harmonization and reduce endpoints we must keep working. Personally I'd prefer if all clients fetched from the git repo, but it's understandable that git can't be expected to be always available. Still, I wonder if there's a way we could use github's auto-archive feature (https://github.com/tldr-pages/tldr/archive/master.zip) for that effect, rather than a custom-made zip file. This should be workable since this repo consists almost exclusively of pages content. We can even consider how to migrate non-pages content elsewhere to avoid polluting the zip file.
What would you suggest? We must use a standardized format for the config file. Markdown or yaml are the most lightweight ones that come to mind, which don't require a sophisticated parser, but there could be options I'm not considering.
Yeah, I believe we're all in agreement that this should be a recommendation, but not a requirement. |
It would be good if there is a shared location among all installed clients, as the data is the same. (Just how expiration is handled might be different.) Because the pages are data, it should then use The most important consideration when using Github for pages retrieval is availability, because of Github's download rate limitations on certain locations but not others. It seems the assets directory on Using YAML is way easier to handle without libraries. |
I'm wondering if, instead of requiring a full content dump download, we could implement a partial update "service" that all clients could use if they don't want to use git -- specifically, a regularly scheduled "release" archive (say once a week) which would be built automatically and placed in the assets directory of Clients could then download that archive, and extract the pages to the cache folder. The archive could even include a log file with all the commit messages from that week, which the client could present to the user to show what's new. This is just off the top of my head -- do you guys think it's workable, or would it be too complex to implement (either in the client or the server side)? |
I think the overhead on the server is almost more if every non-existent/too-old page has to be downloaded separately, since we're looking at a roughly 100kB download for the whole package -- which comes with a consistent & up-to-date index.json. |
@waldyrious - I think its a bit too complicated. |
Throwing my vote in for adhering to the XDG spec for where to place data. The TLDR config file should be under It can be argued that the page data is "data" and be placed under The major sticking point, IMO, is that no client should be considered "official" if it doesn't use XDG-spec env vars for storing the config/data/cache. |
I understand the case for |
@pepa65 generally agree. I think the pages do feel more like data than cache and I might switch my vote on that one.
One thing about leveraging cache is that it virtually eliminates the need to build a feature for wiping/re-fetching. (Since the xdg_cache_home is generally expected to be wiped occasionally) But given that those pages are preferred to be long-lived, (and could conceivably even be delivered as part of the tool's initial installation) makes them fit properly as data. XDG_DATA_HOME for me, now! |
I'd prefer any specification to only describe the interface between the user and the output, and not any implementation. In this spirit I forked the current proposal to remove whatever prescriptions on implementation I could while keeping the spirit of providing a consistent set of expectations and high quality of support: I also added some of the suggestions in this thread that support that spirit. https://ybin.me/p/127d5bfb038aa09e#CGtXFguDi0X+cA2u6/ruEaCwzqIm5xu2lyPlP0Bu/GY= |
A specification is a good to have because it gives guidance for new client authors on how to go about creating a client. Or what are some of the features a client should have. |
I agree that examples are good to have, but I'd rather avoid listing all features every client has done in a specification, and avoid restricting clients to only one way of implementing things. A good example is specifying how files should be stored. What if my client was much better with an internal db? Or was meant to be accessed offline? It would be a shame if we couldn't link to them as 'an officially sanctioned client' just because they implement things differently but are otherwise high quality and behaved consistently with peoples expectations. When I read a specification with so many MAYs, it just feels like a laundry list of all the features any client has done, and I feel obligated to adhere to all the optional stuff or wonder if they are useful. If we can specify a minimum set of unambiguous design features that make a quality client, it helps encourage new clients to be made, possibly with better ideas than we have right now. |
Thanks for the feedback everyone! I've created an updated version of the spec. Here's a link: https://ybin.me/p/dddac3fe0177c4dd#+fRQMkWH7Nuv2geAhX96O8nND2IqzbnICRWDS9+cvc4= I've also responded to individual points raised below:
I'm unsure what you mean by a
Would that be like Personally, I'd prefer something like this: tldr mkdir # Shows page for default platform
tldr windows/mkdir # Shows page in 'windows' platform
Excellent idea.
I'm unsure what you mean. Surely if it's
Sure. The only reason I made it a
So we shouldn't specify the directory to store it in? I see. Never written a 'real' spec before, so thanks for the tip! |
Should it be possible to use two clients on the same machine with expectation that both work totally fine regardless of which client I'm using at a given time. Would a user expect that if they were to use the python, node, and cpp implementations available within the tldr org, they'd end up with a unique cache per client? You could probably put this behind a "If your client utilizes a file based cache, it MUST ..." for the case of clients that use internal DBs. I do agree the number of "SHOULD"/"MAY" be minimized and focus on a core of what you want out of implementations.
This comes down to I guess how you want |
The 'one page per line' use case seems the simplest, I'd be happy to see that be the default behavior for @MasterOdin I would prefer clients that allow you to point them towards a cache dir, than the organization dictate where it should be. But I don't think people making clients should be forced to support configurable cache directories, as that adds friction to development and just plain might not make sense. By not forcing a particular directory, we are making it easier for multiple clients to coexist, at the cost of on-disk duplication. Though I don't think its a great idea, it also allows for clients to do other things in their own directory. In addition there are differing opinions on where files should go - for example the XDG spec, or I put some files on a separate volume to work better with backup software. If tldr-pages forced it to be in my home directory, I'd need to hack around it with symlinks which might break some clients or my backup software, etc. Though my philosophy for specifications of this nature are "Does this help specify the minimum criteria for developing a solid tldr client?" and hard-coding a location for where things should live on-disk seems over that bounds. If we are trying to show examples of good cli design, I could maybe see:
Or something of that nature |
Yes, if you see my earlier comment, this is exactly what I wanted.
In the node client, it is
Sorry, I wasn't clear maybe. I was saying we should specify the directory. But the internal implementation of the the cache is managed should be left upto the client. |
I see. I'm unsure about that comma-separated output.
...I see. What do you think of the alternative
I'm unsure that such an option should be specified via a command-line argument. I suspect that it's better suited to an environment variable, such as I definitely think we should not specify how the cache should be stored - only where. e.g. it could be stored in a leveldb, or an sqlite database, etc. |
The easiest, most accessible and portable way to store the cache is to unpack the archive containing index.json and the platform directories containing the pages. If you stick to POSIX, you should put it in |
tl;dr I think the only thing the spec should recommend regarding how and where files should be stored, is that clients provide a way to specify where files should be stored in I'm getting confused about the difference people are using the words how and where things are stored, and how to configure where things should be stored. Here's my understanding:
Now some thoughts on what the spec should say on these things
What I expect will be the most common implementation:
I think one of the trickiest things about this, is that tldr uses Oh and I chose LOCATION because CACHE{DIRECTORY,PATH, FOLDER} are called different things on different platforms (and even within different file managers, for instance), but I really don't care what its called as long as the spec recommends that it is configurable. |
Excellent demystification, @jedahan :D |
I think you meant to say "we should not" 😆
I don't understand. What is the alternative syntax ? |
Yeah, I did! I'll edit that comment. Here's a comparison of the 2 syntaxes: tldr --os windows mkdir
tldr windows/mkdir Edit: We should probably also mention that failures MUST result in a non-0 exit code, if appropriate. |
I see. But in both cases, the client needs to be updated right ? That was my point. Unless you are saying that Also, sometimes the default host platform does not exactly match with the folder name (the case without |
I just implemented platform-agnosticism in the tldr-bash-client (meaning no hardcoded platforms, whatever directories are in the archive will be allowed), and that is no problem. Of course the detection of the current platform is still dependent on the detection code, but new directories in the archive will be usable without the code needing to be updated. |
The platform names don't correspond to the folder names? |
I would hope the archive will keep having folder names that correspond to the platform names. So far that is the case. (Except for |
I mean it depends on the client, the programming language, and where it's being run. Take osx for eg. To detect the host OS, the client might get osx, darwin, macOS and all will be correct. But the folder name is osx. |
@agnivade Oh, right! I never knew. I don't actually have a mac machine, so I have no idea as to how it identifies itself :P So what should we put for the platform specification in the spec? |
Whatever is fine. I don't have an opinion on that. I just wanted to say that the client needs to be updated. |
It's finally happened! See #2706 for the full specification I've drafted up. Taken me all afternoon, but worth it 😺 🎆 |
Background:
--update
example #744The text was updated successfully, but these errors were encountered: