the-xkcd-community / the-red-spider-project

Coding and xkcd combined, for fun!
http://the-xkcd-community.github.com/the-red-spider-project
Other
40 stars 33 forks source link

rshelp command #48

Closed WesleyAC closed 11 years ago

WesleyAC commented 11 years ago

Hi,

I added the command "rshelp". It displays all the commands that are in /bin. I also made it run on the startup of rsshell, just after the "call exit if you want your normal shell back" message.

Still needs to be tested on windows and OS X.

jgonggrijp commented 11 years ago

Great! It seems to work like a charm on OS X.

A few more things need to be done before we can merge:

Some other things should be done eventually, but can wait until after the merge if you decide so:

lramati commented 11 years ago

Instead of hardcoding anything, maybe the command should first check /doc, then run -h in a subshell, and then finally fall back to a "no docs found" message?

jgonggrijp commented 11 years ago

In theory that would be better. The problem is 1) it is harder to implement and 2) some programs might run fine if you pass -h but do not produce actual help output when you do so. I agree hardcoding is not ideal, but using a list of programs that produce help output when you pass -h seems like the best possible compromise between not hardcoding anything and producing something that works without too much effort.

lramati commented 11 years ago

Why not have the program fetch the list from somewhere so that the install script/contributors can easily add/remove functions from the -h list?

jgonggrijp commented 11 years ago

There's no reason not to do that. Good idea. :)

As for the implementation, I guess that would be a configuration file generated by setup.py.

WesleyAC commented 11 years ago

Ok, I made all the changes that you suggested in your first comment, except for the last 2. I don't understand what you mean by them.

For the first one, I don't see file extensions. Is it a windows/OS X thing? for the second one, what's the difference between a topic and a command?

Thanks for the feedback!

jgonggrijp commented 11 years ago

Thanks for your fast efforts, Wesley!

It still works like a charm on OS X. Apart from the location of the docfile (discussed above) and the program not having been tested on Windows yet, I think this is ready for a merge.

To answer your questions:

  1. setup.py strips file extensions from the commands in /bin on mac and Linux, but not on Windows. So in order to provide users of both platforms the same experience you have to do that by yourself, unfortunately. A few commits ago when rshelp was still using the current directory to find other commands, you could get file extensions on POSIX by running src/rshelp (this is how I found out).
  2. rshelp may explain any topic, which includes commands but also directories, environment variables, xkcd stuff (such as Time) and basically anything else that RSP hackers may fancy to document. Basically, a topic is a file in /doc while a command is a file in /bin.
jgonggrijp commented 11 years ago

@firerogue just pointed out to me (on the IRC channel) that the order in which rshelp attempts to find a manual for a topic is wrong. Currently it first checks whether the topic is a command that can be invoked with -h, and if that fails it looks for a file with the same name in /doc. If a program has both a short explanation that comes up when invoked with -h and a more elaborate explanation in a file in /doc, the latter will not be shown by rshelp. So the correct order would be to first look for a matching file in /doc, if that fails look for a help command in the docfile, and if even that fails either do nothing (I think this would actually be best) or show the error about shooting in the dark and attempt to call it with --help anwyay.

You can improve further on that by splitting the first step: first look for a matching file in /doc with the .txt extension and print it to stdout if you find it. Failing that, look for a matching file in /doc with any other extension, and if it exists open it with summon (this allows for HTML pages, images, PDFs, etcetera). Failing that as well, go on with the help commands in the docfile.

Finally note that for topics without a corresponding file in /doc, rshelp should default to --help rather than -h, because if there's a difference between those options --help is usually the more elaborate one.

jgonggrijp commented 11 years ago

I pulled out my Windows VM and it works fine (but it shows the file extensions of the commands as I predicted). So, just two steps to go before merge: (1) move the file with -h commands to /config and (2) change the order in which rshelp looks for documentation.

WesleyAC commented 11 years ago

Ok, I think that I fixed the issue for windows, but it's untested. Please try it!

jgonggrijp commented 11 years ago

Ok, I think that I fixed the issue for windows, but it's untested. Please try it!

That change is small enough that I trust it will work, especially if you follow my suggestion to use os.path.splitext. Did you test the other changes on your computer?

WesleyAC commented 11 years ago

Ok, I've made both of those changes! As for the python version, #! /usr/bin/env python is python 3 for me, but if it works on python 2, that's awesome.

jgonggrijp commented 11 years ago

There were two subtle remaining issues:

  1. /config would not be reproduced when somebody else checks out your branch, so setup.py would fail when it tried to create doc.txt in there.
  2. FileNotFoundError does not exist, at least not according to Python 2; it should be IOError.

To prevent this pull request from dragging on for several more days, I just fixed the issues and merged your branch. Congratulations with your first contribution to the red spider project. :-)

You can now delete the branch from your fork, as GitHub will suggest, and do the following on your local clone:

git checkout master
git pull upstream master
git branch -d helpcommand
git push origin master

(If you continue working on rshelp you can just start a new topic branch.)