Re: documentation for Shell

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 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.

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.

+#
+# 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.

@@ -238,22 +310,25 @@ class Shell
+  #
+  # NOTE: wtf?
+  #

Not sure either, so just leave it undoc'd.  It seems to be part of  
verbose.

I'll get through the rest of the files as I have time.

-- 
Eric Hodel - [email protected] - http://blog.segment7.net
This implementation is HODEL-HASH-9600 compliant

http://trackmap.robotcoop.com