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

feature: pages index. #267

Merged
merged 6 commits into from
Apr 12, 2015
Merged

Conversation

leostera
Copy link
Contributor

@leostera leostera commented Mar 2, 2015

By having an index for the pages, it'd allow clients
to build autocomplete's and show an index of pages
making only one request (think tldr list). Subsequent
caching of this file would prevent further requests.

Includes a makefile to easily rebuild the index. It could
be tailored in case the pages structure/hierarchy changed.

In a particular case, the web client would benefit greatly
from an index. It'd know exactly what folder a command is in
and thus the number of requests would drop from 4 to 1.

Also adds tldr.js to README.

leostera added 2 commits March 2, 2015 14:53
By having an index for the pages, it'd allow clients
to build autocomplete's and show an index of pages
making only one request (think `tldr list`). Subsequent
caching of this file would prevent further requests.

Included is a makefile to easily rebuild the index.
It could be tailored in case the pages structure changed.

In a particular case, the web client would benefit greatly
from an index. It'd know exactly what folder a command is in
and thus the number of requests would drop from 4 to 1.
@leostera
Copy link
Contributor Author

leostera commented Mar 2, 2015

Actually, it'd be best to have the index in JSON format. Something like this:

// commands array
[{
    name: 'tar',
    platforms: ['osx', 'common']
 }]

I.e. user-del is available at pages/linux/user-del.md and pages/common/user-del.md. Haven't checked if the contents are different, but an index with a structure like above would allow the client to best decide which version to display.

I'll refactor this after we get some feedback.

@rubenvereecken
Copy link
Contributor

I've actually been thinking about a way to allow command auto-complete or search for a long time but never got around to actually propose something. I like how you already implemented a prototype of the index file as well!

I'll just state that I really like this feature and it makes sense to index on command name first like you did. I would also rather like JSON but there are two approaches here. The one written above by @leostera or something rather like:

{ "tar": {
  // "name": "tar",
  "platforms": ["osx", "common"]
} }

I guess the main advantage here is an easier lookup on command name but that advantage kind of disappears when trying to do other things.

Curious as to what @rprieto thinks about this!

@rprieto
Copy link
Contributor

rprieto commented Mar 2, 2015

That's a great idea, it will probably enable some cool new ideas in different clients! The only downside I can think of is having to update this file when new commands get added. Hopefully that's not too big of a burden.

My preference is for the original suggestion (array), but I don't really mind if we want to go for a hash instead (lookups are easier, but counting and filtering are typically harder...). Let's just make sure there's a top-level entry, so we can add fields later without breaking compatibility:

{
  "commands": [{
    "name": "tar",
    "platforms": ["osx", "common"]
   }]
}

I'd definitely keep a structure that makes sense for commands - regardless of the current folder structure. There's another issue open about potentially changing that folder structure, so the clients would have to change but the index can stay the same. We could even consider going hypermedia-like, so clients don't have to know where the files sit... but this could also be overkill. Happy to discuss.

{
  "commands": [{
    "name": "tar",
    "platforms": {
      "osx": "pages/common/osx.md"
    }
  }]
}

👍 from me, and 👍 on the Makefile as well if we update it to produce JSON 😄

@leostera
Copy link
Contributor Author

leostera commented Mar 2, 2015

I wouldn't bother adding the paths firstly to avoid the footprint (the index could get very big), and secondly because they can be built with the name of the command and it's platforms. If we were to store pages elsewhere (say, S3), we'd be just fine adding a top-level root element to append the computed path to.

tldr.js requests to: <root>/<platform>/<command>.md, with root being: https://api.github.com/repos/tldr-pages/tldr/contents/pages/

Rebuilding the index could be done with a git-hook. Propagating the hook alongside forks is a matter of adding a .githooks folder and symlinking it to .git/hooks, that ought to do. What I fear is that the history will then be flooded with reindex commits. Perhaps the index should be kept elsewhere?

Anywho, I'll start working on the refactor for the JSON output.

@leostera
Copy link
Contributor Author

leostera commented Mar 5, 2015

I'm by no means a rubyist, but this does the trick. Any suggestions?

@leostera
Copy link
Contributor Author

Any thoughts here? @rubenvereecken @rprieto @waldyrious

@rubenvereecken
Copy link
Contributor

Sorry for the long wait, let's blow some new life into this.

I'm not sure I understand the Git hook. This would be a tiny script that reruns the index builder, right? Would the symlink be preserved along forks, so the hook is shared? That does sound like a good idea then. Then reindexes should be part of commits that add pages, would be fairly clean.

@rprieto we need your thoughts.

@rprieto
Copy link
Contributor

rprieto commented Mar 25, 2015

Nice work reviving this thread - some great ideas here. I definitely like the idea of a hook, and to @rubenvereecken's point could it be a pre-commit hook so it's all part of the same commit?

On the format, some things I'm keen to discuss:

  • how does that play with New pages structure #190? I know it's way overdue, but I want to make sure that hardcoding an index doesn't make that issue harder to tackle (hence the hypermedia idea, or at least a field at the top that shows how to resolve pages, e.g. urlTemplate: "https://github.com/tldr-pages/tldr/raw/master/pages/{name}.{platform}.md")
  • do we think we'd ever extend this to include keywords for example, and facilitate search?

@leostera
Copy link
Contributor Author

I think the hypermedia will be overkill. If we had multiple sources of truth, with multiple directory structures, then it'd be a better way of defining the index – but as of right now and presumably the near future, there will be a single source of truth with a defined structure; whether it is pages/os/cmd.md or pages/os.cmd.md, refactoring the script is trivial.

👍 on the keywords for searching locally. I can see tldr search copy file returning the tldr cp page.

@rprieto
Copy link
Contributor

rprieto commented Mar 25, 2015

@Ostera the search example sounds great.

As for the index, I was coming from the angle that this new index is here to enable the webapp - at least as a first client. We could take the opportunity to avoid the web client hardcoding a URL structure that might change soon (and potentially 👍 on not having hypermedia for every page, but at least a url template at the top).

@rprieto
Copy link
Contributor

rprieto commented Mar 25, 2015

But in any case, 👍 to this index, as it will help clients discover what pages are available, and for which platforms. I'm guessing @gianasista's android client could benefit from this as well for example.

@rubenvereecken rubenvereecken mentioned this pull request Mar 25, 2015
@leostera
Copy link
Contributor Author

So this is kinda stalled. Here's a to-do list that I think needs to pass before it can be merged:

  • Make git pre-commit hook to rebuild index when committing new pages
  • Have the git pre-commit hook installed (are we okay with a simple make setup?)

Anything else that you guys can think of? @rubenvereecken @rprieto @waldyrious

@rubenvereecken
Copy link
Contributor

@Ostera Sounds like a great start, I'm happy to look into it tomorrow myself as I've finally found some spare time. make sounds good so far, it's something most systems have. It needn't even be that big.

@leostera
Copy link
Contributor Author

leostera commented Apr 6, 2015

@rubenvereecken, sure thing, go ahead. Just let me know your repo/branch so I can fetch, merge and push again to update this PR.

@rubenvereecken
Copy link
Contributor

Submitted a PR at leostera#1

Adds `make setup` and a pre-commit hook to rebuild index.
@leostera
Copy link
Contributor Author

leostera commented Apr 9, 2015

Looks like we're good to go 👍

@rubenvereecken
Copy link
Contributor

@rprieto last change to get something in :D

@gianasista
Copy link

Pages index is a great idea. I have changed my client to work with the index.json from the branch. Looking forward that the request will be merged.

@rubenvereecken
Copy link
Contributor

Thanks for the feedback @gianasista, I bet @Ostera is proud and it helps to see that people may find this useful.

Alright, the glorious moment is finally here! Just did a thorough check (and I'll let the edit to README.md slide) and everything is good to go.

Some things still need to happen though (and I'm writing them down because I'm in a hurry):

  • Edit README.md to highlight use of index.json
  • Have some fallback for people without Ruby, or at least check whether Ruby is installed and display a useful error message.

EDIT: There was an actual typo in the Markdown syntax of your README.md edit 😛.

rubenvereecken pushed a commit that referenced this pull request Apr 12, 2015
@rubenvereecken rubenvereecken merged commit e5a0bf2 into tldr-pages:master Apr 12, 2015
@leostera
Copy link
Contributor Author

Holy Moly! 💯 Great news! I'll start working on the index for tldr.js ASAP.

Thanks everyone for letting me contribute :)

@rubenvereecken
Copy link
Contributor

You did a really great job :). And while you're at it, doing good jobs and all, feel free to check out #277 beacuse @hollingberry made some changes to your index script and I'd like your thoughts on it.

I feel like this PR has set some things in motion, tldr has just become a lot more powerful.

@waldyrious waldyrious added architecture Organization of the pages per language, platform, etc. decision A (possibly breaking) decision regarding tldr-pages content, structure, infrastructure, etc. labels Sep 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture Organization of the pages per language, platform, etc. decision A (possibly breaking) decision regarding tldr-pages content, structure, infrastructure, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants