Re: documentation for Shell

On 10/15/06, Eric Hodel <[email protected]> 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 - [email protected] - http://blog.segment7.net
> This implementation is HODEL-HASH-9600 compliant
>
> http://trackmap.robotcoop.com

New patch for that file attached.

-- 
Konrad Meyer

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	16 Oct 2006 05:52:27 -0000
@@ -1,11 +1,15 @@
+#--
+#   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 - 
-#   	$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
 #
-#   
+# Rubifies working with a local shell. See Shell for documentation, and
+# Net::Telnet for working with remote shells.
 #
@@ -19,2 +23,14 @@ require "shell/process-controller"
 
+# 
+# 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 +56,9 @@ class Shell
 
+    # 
+    # Sets the debug level to +val+ and enables verbose when +val+ is a
+    # true value.
+    #
+    # If +val+ is literally true, we get normal debug. A +val+ of 1 is "eval
+    # definition debug", and a +val+ of 2 is "detail inspect debug".
+    #
     def debug=(val)
@@ -45,2 +68,6 @@ class Shell
 
+    # 
+    # Creates a new Shell, changes its working directory to +path+, and
+    # returns the new shell object.
+    #
     def cd(path)
@@ -51,2 +78,5 @@ class Shell
 
+    # 
+    # Returns the default executable search path.
+    #
     def default_system_path
@@ -59,2 +89,6 @@ class Shell
 
+    # 
+    # Sets the default executable search path to +path+. +path+ should be
+    # Enumerable.
+    #
     def default_system_path=(path)
@@ -63,2 +97,5 @@ class Shell
 
+    # 
+    # Returns the default record separator or $/.
+    #
     def default_record_separator
@@ -71,2 +108,5 @@ class Shell
 
+    # 
+    # Sets the default record separator to +rs+.
+    #
     def default_record_separator=(rs)
@@ -76,2 +116,5 @@ class Shell
 
+  # 
+  # Creates a new Shell object.
+  #
   def initialize
@@ -93,2 +136,5 @@ class Shell
 
+  # 
+  # Sets the system path, and flushes the command list.
+  #
   def system_path=(path)
@@ -101,2 +147,6 @@ class Shell
 
+  # 
+  # Sets debugging level to +val+; if +val+ is true, enables verbose mode.
+  # See also: Shell::debug=.
+  #
   def debug=(val)
@@ -112,2 +162,6 @@ class Shell
 
+  # 
+  # Expands a relative +path+ to a full path, relative to the working
+  # directory.
+  #
   def expand_path(path)
@@ -116,14 +170,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 +178,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 +201,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, +path+ is pushed onto
+  # the directory stack, +self+ is yielded to the block, and popdir is
+  # called when the block finishes.
+  #
+  # Example:
+  #   require 'shell'
+  #
+  #   Shell.new.pushdir("/etc") { |sh| ... }
+  #
   def pushdir(path = nil)
@@ -183,2 +239,5 @@ class Shell
 
+  # 
+  # Pop the top directory off of the directory stack and chdir to it.
+  #
   def popdir
@@ -195,5 +254,5 @@ class Shell
 
-  #
-  # process management
-  #
+  # 
+  # Returns an Array of active and waiting jobs. See ProcessController#jobs.
+  # 
   def jobs
@@ -202,2 +261,5 @@ class Shell
 
+  # 
+  # Sends signal +sig+ to +command+.
+  #
   def kill(sig, command)
@@ -207,3 +269,3 @@ class Shell
   #
-  # command definitions
+  # Defines a system command. See CommandProcessor::def_system_command.
   #
@@ -213,2 +275,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 +283,5 @@ class Shell
 
+  # 
+  # Creates an alias to a command. See CommandProcessor::alias_command.
+  #
   def Shell.alias_command(ali, command, *opts, &block)
@@ -221,2 +290,6 @@ class Shell
 
+  # 
+  # Removes an alias to a command; opposite of alias_command. See
+  # CommandProcessor::unalias_command.
+  #
   def Shell.unalias_command(ali)
@@ -225,2 +298,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_")
@@ -229,2 +306,4 @@ class Shell
 
+  # 
+  # :nodoc:
   #
@@ -238,22 +317,25 @@ class Shell
 
+  # 
+  # :nodoc:
+  #
   def self.notify(*opts, &block)
     Thread.exclusive do
-    if opts[-1].kind_of?(String)
-      yorn = verbose?
-    else
-      yorn = opts.pop
-    end
-    return unless yorn
-
-    _head = true
-    print opts.collect{|mes|
-      mes = mes.dup
-      yield mes if iterator?
-      if _head
-	_head = false
-	"shell: " + mes
+      if opts[-1].kind_of?(String)
+	yorn = verbose?
       else
-	"       " + mes
+	yorn = opts.pop
       end
-    }.join("\n")+"\n"
+      return unless yorn
+
+      _head = true
+      puts opts.collect{|mes|
+	mes = mes.dup
+	yield mes if iterator?
+	if _head
+	  _head = false
+	  "shell: " + mes
+	else
+	  "       " + mes
+	end
+      }.join("\n")
     end