HBASE-20276 restore original shell REPL functionality where commands can return results

* makes commands always pass any results back to hirb
* print warning if hirb is given the --return-values flag
* add some docs on how to avoid the console clutter that HBASE-15965 sought to address
* add an upgrade section note about this change.
* cleanup where the get_splits command does its printing so there's a building block that doesn't print
* some rubocop suggested tweaks and opt-out for classlength check on table and shell classes.

Signed-off-by: Mike Drob <mdrob@apache.org>

bin/hirb.rb

@@ -59,15 +59,12 @@ Usage: shell [OPTIONS] [SCRIPTFILE [ARGUMENTS]]
  -n | --noninteractive          Do not run within an IRB session
                                 and exit with non-zero status on
                                 first error.
- -r | --return-values           Include return values from commands
-                                executed in the shell.
 HERE
 found = []
 script2run = nil
 log_level = org.apache.log4j.Level::ERROR
 @shell_debug = false
 interactive = true
-return_values = false
 for arg in ARGV
   if arg == '-h' || arg == '--help'
     puts cmdline_help
@@ -82,7 +79,8 @@ for arg in ARGV
     interactive = false
     found.push(arg)
   elsif arg == '-r' || arg == '--return-values'
-    return_values = true
+    warn '[INFO] the -r | --return-values option is ignored. we always behave '\
+         'as though it was given.'
     found.push(arg)
   else
     # Presume it a script. Save it off for running later below
@@ -116,7 +114,7 @@ require 'shell/formatter'
 @hbase = Hbase::Hbase.new
 
 # Setup console
-@shell = Shell::Shell.new(@hbase, interactive, return_values)
+@shell = Shell::Shell.new(@hbase, interactive)
 @shell.debug = @shell_debug
 
 # Add commands to this namespace

hbase-shell/src/main/ruby/hbase/table.rb

@@ -19,9 +19,12 @@
 
 include Java
 
+java_import org.apache.hadoop.hbase.util.Bytes
+
 # Wrapper for org.apache.hadoop.hbase.client.Table
 
 module Hbase
+  # rubocop:disable Metrics/ClassLength
   class Table
     include HBaseConstants
     @@thread_pool = nil
@@ -804,12 +807,12 @@ EOF
     # Get the split points for the table
     def _get_splits_internal
       locator = @table.getRegionLocator
-      splits = locator.getAllRegionLocations
-                      .map { |i| Bytes.toStringBinary(i.getRegionInfo.getStartKey) }.delete_if { |k| k == '' }
+      locator.getAllRegionLocations
+             .map { |i| Bytes.toStringBinary(i.getRegionInfo.getStartKey) }
+             .delete_if { |k| k == '' }
+    ensure
       locator.close
-      puts(format('Total number of splits = %s', splits.size + 1))
-      puts splits
-      splits
     end
   end
+  # rubocop:enable Metrics/ClassLength
 end

hbase-shell/src/main/ruby/shell.rb

@@ -68,22 +68,18 @@ module Shell
   end
 
   #----------------------------------------------------------------------
+  # rubocop:disable Metrics/ClassLength
   class Shell
     attr_accessor :hbase
     attr_accessor :interactive
-    attr_accessor :return_values
     alias interactive? interactive
-    alias return_values? return_values
 
     @debug = false
     attr_accessor :debug
 
-    def initialize(hbase, interactive = true, return_values = !interactive)
+    def initialize(hbase, interactive = true)
       self.hbase = hbase
       self.interactive = interactive
-      self.return_values = return_values
-      # If we're in non-interactive mode, force return_values
-      self.return_values = true unless self.interactive
     end
 
     # Returns Admin class from admin.rb
@@ -140,11 +136,8 @@ module Shell
     end
 
     # call the method 'command' on the specified command
-    # If return_values is false, then we suppress the return value. The command
-    # should have printed relevant output.
     def command(command, *args)
-      ret = internal_command(command, :command, *args)
-      ret if return_values
+      internal_command(command, :command, *args)
     end
 
     # call a specific internal method in the command instance
@@ -245,6 +238,7 @@ For more on the HBase Shell, see http://hbase.apache.org/book.html
       HERE
     end
   end
+  # rubocop:enable Metrics/ClassLength
 end
 
 # Load commands base class

hbase-shell/src/main/ruby/shell/commands/get_splits.rb

@@ -37,7 +37,11 @@ EOF
       end
 
       def get_splits(table)
-        table._get_splits_internal
+        splits = table._get_splits_internal
+        puts(format('Total number of splits = %<numsplits>d',
+                    numsplits: (splits.size + 1)))
+        puts splits
+        splits
       end
     end
   end

hbase-shell/src/test/ruby/shell/noninteractive_test.rb

@@ -20,7 +20,7 @@ require 'shell'
 class NonInteractiveTest < Test::Unit::TestCase
   def setup
     @hbase = ::Hbase::Hbase.new($TEST_CLUSTER.getConfiguration)
-    @shell = Shell::Shell.new(@hbase, false, true)
+    @shell = Shell::Shell.new(@hbase, false)
   end
 
   define_test "Shell::Shell noninteractive mode should throw" do

hbase-shell/src/test/ruby/test_helper.rb

@@ -43,7 +43,7 @@ module Hbase
 
     def setup_hbase
       hbase = ::Hbase::Hbase.new($TEST_CLUSTER.getConfiguration)
-      @shell = ::Shell::Shell.new(hbase, interactive = false, return_values = true)
+      @shell = ::Shell::Shell.new(hbase, interactive = false)
     end
     
     def shutdown

src/main/asciidoc/_chapters/shell.adoc

@@ -318,6 +318,7 @@ hbase(main):017:0> tables.map { |t| disable t ; drop  t}
 hbase(main):018:0>
 ----
 
+[[irbrc]]
 === _irbrc_
 
 Create an _.irbrc_ file for yourself in your home directory.
@@ -331,6 +332,13 @@ IRB.conf[:SAVE_HISTORY] = 100
 IRB.conf[:HISTORY_FILE] = "#{ENV['HOME']}/.irb-save-history"
 ----
 
+If you'd like to avoid printing the result of evaluting each expression to stderr, for example the array of tables returned from the "list" command:
+
+[source,bash]
+----
+$ echo "IRB.conf[:ECHO] = false" >>~/.irbrc
+----
+
 See the `ruby` documentation of _.irbrc_ to learn about other possible configurations.
 
 === LOG data to timestamp

src/main/asciidoc/_chapters/upgrading.adoc

@@ -480,10 +480,12 @@ Previously, the Web UI included functionality on table status pages to merge or
 
 User running versions of HBase prior to the 1.4.0 release that make use of replication should be sure to read the instructions in the section <<upgrade1.4.replication>>.
 
-[[upgrade2.0.jruby]]
-.HBase shell now based on JRuby 9.1.10.0
+[[upgrade2.0.shell]]
+.HBase shell changes
 
-The bundled JRuby 1.6.8 has been updated to version 9.1.10.0. The represents a change from Ruby 1.8 to Ruby 2.3.3, which introduces non-compatible language changes for user scripts.
+The HBase shell command relies on a bundled JRuby instance. This bundled JRuby been updated from version 1.6.8 to version 9.1.10.0. The represents a change from Ruby 1.8 to Ruby 2.3.3, which introduces non-compatible language changes for user scripts.
+
+The HBase shell command now ignores the '--return-values' flag that was present in early HBase 1.4 releases. Instead the shell always behaves as though that flag were passed. If you wish to avoid having expression results printed in the console you should alter your IRB configuration as noted in the section <<irbrc>>.
 
 [[upgrade2.0.coprocessors]]
 .Coprocessor APIs have changed in HBase 2.0+