On 10/15/06, Eric Hodel <drbrain@xxxxxxxxxxxx> wrote:
On Oct 7, 2006, at 10:54 PM, Konrad Meyer wrote:
> See attached; I tried to make sure there was at least a one line
> explanation of all methods, but I might have made mistakes, done
> something stupid, whatever. Comments *greatly* welcome. In some places
> I fixed whitespace (for consistancy), but mostly left it alone.
Can you leave the whitespace alone? or would it be too much work?
I don't think I actually changed anything. If you notice something,
tell me, but I don't remember anything and don't know why I wrote
that.
> I believe there were two methods like:
>
> def foo
> @foo
> end
>
> that I changed to attr_reader :foo. That's all, please, feedback!
Keep the $Blah$s for all files.
Indeed, changed.
Index: shell.rb
===================================================================
RCS file: /src/ruby/lib/shell.rb,v
retrieving revision 1.9
diff -p -u -1 -r1.9 shell.rb
--- shell.rb 20 Jul 2006 17:35:26 -0000 1.9
+++ shell.rb 8 Oct 2006 05:47:06 -0000
@@ -1,11 +1,9 @@
#
-# shell.rb -
-# $Release Version: 0.6.0 $
-# $Revision: 1.8 $
-# $Date: 2001/03/19 09:01:11 $
-# by Keiju ISHITSUKA(Nippon Rational Inc.)
+# = shell.rb: Emulate a system shell
#
-# --
+# Author:: Keiju Ishitsuka
+# Documentation:: Konrad Meyer
#
-#
+# Rubifies working with a local shell. See Shell for documentation, and
+# Net::Telnet for working with remote shells.
#
@@ -19,2 +17,15 @@
require "shell/process-controller"
These requires need to be above the documentation.
Whenever I do this, stdlib-doc neglects the file-level documentation.
So, could you clarify or something as to how you want that?
+#
+# Shell is the parent class for working with the ruby representation
of the
+# shell.
+#
+# Directory related methods:
+# Shell#cwd (or #dir, #getwd, or #pwd),
+# Shell#chdir (or #cd),
+# Shell#pushdir (or #pushd),
+# Shell#popdir (or #popd),
+# Shell#mkdir, and
+# Shell#rmdir.
+#
+
class Shell
@@ -40,2 +51,6 @@ class Shell
+ #
+ # If +val+ evaulates to true, turns on verbose. In all cases, sets
+ # @debug to +debug+.
+ #
def debug=(val)
+debug+ -> +val+
How about "Sets the debug level to +val+ and enables verbose when +val
+ is a true value."
Can the comments above @debug just below class Shell be moved here too?
@@ -51,2 +70,6 @@ class Shell
+ #
+ # Returns @default_system_path, or if that isn't set, the
+ # environmental variable _PATH_ split into an array.
+ #
def default_system_path
How about "Returns the default executable search path"?
Do we need "split into an array"?
@@ -59,2 +82,5 @@ class Shell
+ #
+ # Setter for @default_system_path.
+ #
def default_system_path=(path)
How about "Sets the default executable search path to +path+"?
Should the type of argument be listed here? I assume only an
Enumerable is acceptable.
@@ -63,2 +89,5 @@ class Shell
+ #
+ # Returns @default_record_separator, or if that isn't set, $/.
+ #
def default_record_separator
How about "Returns the default record separator or $/"?
@@ -71,2 +100,5 @@ class Shell
+ #
+ # Sets the value of @default_record_separator.
+ #
def default_record_separator=(rs)
How about "Sets the default record separator to +rs+"?
@@ -76,2 +108,5 @@ class Shell
+ #
+ # Creates a new Shell object.
+ #
def initialize
@@ -93,2 +128,5 @@ class Shell
+ #
+ # Sets the system path, and rehashes.
+ #
def system_path=(path)
"and rehashes" isn't very descriptive. I don't know of a good way to
say what it does though...
"and flushes the system command list" ?
@@ -101,2 +139,6 @@ class Shell
+ #
+ # Turns on debugging if +val+ evaluates to true. Also turns on
verbose if
+ # +val+ evaluates to true.
+ #
def debug=(val)
If you add the debug level info to Shell::debug= could you add a see
also here?
@@ -112,2 +154,5 @@ class Shell
+ #
+ # Expands a relative +path+ to a full path, using File.expand_path.
+ #
def expand_path(path)
How about replacing ", using ..." with "relative to the working
directory"?
@@ -116,14 +161,2 @@ class Shell
- # Most Shell commands are defined via CommandProcessor
-
- #
- # Dir related methods
- #
- # Shell#cwd/dir/getwd/pwd
- # Shell#chdir/cd
- # Shell#pushdir/pushd
- # Shell#popdir/popd
- # Shell#mkdir
- # Shell#rmdir
-
attr :cwd
@@ -136,4 +169,6 @@ class Shell
- # If called as iterator, it restores the current directory when the
- # block ends.
+ #
+ # Changes working directory. If passed a block, it restores the
+ # current directory when the block ends.
+ #
def chdir(path = nil)
@@ -157,2 +192,14 @@ class Shell
+ #
+ # Push current directory on to the directory stack, and change to the
+ # directory +path+. If +path+ isn't given, pop the topmost directory
+ # stack entry, and go there. If a block is given, +self+ is
yielded to
+ # the block after +path+ is pushed, and once the block finishes,
popdir
+ # is called.
+ #
+ # Example:
+ # require 'shell'
+ #
+ # Shell.new.pushdir("/etc") { |sh| ... }
+ #
def pushdir(path = nil)
For the last sentence, how about:
If a block is given +path+ is pushed onto the directory stack, +self+
is yielded to the block, and #popdir is called when the block finishes.
@@ -183,2 +230,5 @@ class Shell
+ #
+ # Pop the top directory off of the directory stack and chdir to it.
+ #
def popdir
@@ -195,5 +245,6 @@ class Shell
- #
- # process management
- #
+ #
+ # Returns active and waiting jobs in an array. See
+ # ProcessController#jobs.
+ #
def jobs
I like "Returns an Array of active and waiting jobs." This sets up
links properly.
@@ -202,2 +253,5 @@ class Shell
+ #
+ # Kills a job, with signal +sig+ if applicable.
+ #
def kill(sig, command)
The arguments appear mandatory. Would "Sends signal +sig+ to +command
+." be more descriptive?
@@ -207,3 +261,3 @@ class Shell
#
- # command definitions
+ # Defines a system command. See CommandProcessor.def_system_command.
#
@@ -213,2 +267,6 @@ class Shell
+ #
+ # Undefines a system command. Opposite of def_system_command. See
+ # CommandProcessor.undef_system_command.
+ #
def Shell.undef_system_command(command)
@@ -217,2 +275,5 @@ class Shell
+ #
+ # Creates an alias to a command. See CommandProcessor.alias_command.
+ #
def Shell.alias_command(ali, command, *opts, &block)
@@ -221,2 +282,6 @@ class Shell
+ #
+ # Removes an alias to a command; opposite of alias_command. See
+ # CommandProcessor.unalias_command.
+ #
def Shell.unalias_command(ali)
You've got a ; before opposite of here and a new sentence for
undef_system_command.
@@ -225,2 +290,6 @@ class Shell
+ #
+ # Installs system commands with prefix +pre+. +pre+ defaults to
"sys_".
+ # See CommandProcessor.install_system_commands.
+ #
def Shell.install_system_commands(pre = "sys_")
For all five of these can you use :: instead of . for the "See ..."?
@@ -229,2 +298,5 @@ class Shell
+ #
+ # If debugging is on higher than 2, calls super. Otherwise, it just
+ # runs to_s.
#
I think # :nodoc: is fine for #inspect.
OK, fixed all those bits up to here.
@@ -238,22 +310,25 @@ class Shell
+ #
+ # NOTE: wtf?
+ #
Not sure either, so just leave it undoc'd. It seems to be part of
verbose.
Oops, didn't mean to actually leave that in.
I'll get through the rest of the files as I have time.
--
Eric Hodel - drbrain@xxxxxxxxxxxx - http://blog.segment7.net
This implementation is HODEL-HASH-9600 compliant
http://trackmap.robotcoop.com
New patch for that file attached.
--
Konrad Meyer
shell.rb.patch
Description: Text Data
|