[GitHub] [hbase] ndimiduk commented on a change in pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

[GitBox <gi...@apache.org>]

ndimiduk commented on a change in pull request #2232:
URL: https://github.com/apache/hbase/pull/2232#discussion_r468040659



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -335,12 +348,9 @@ def eval_io(io)
         # Include the 'ERROR' string to try to make transition easier for scripts that
         # may have already been relying on grepping output.
         puts "ERROR #{e.class}: #{message}"
-        if $fullBacktrace
-          # re-raising the will include a backtrace and exit.
-          raise e
-        else
-          exit 1
-        end
+        raise e unless hide_traceback
+
+        exit 1

Review comment:
       Control flow returns to this line after a `raise` ? `raise` is an expression in this language..?

##########
File path: hbase-shell/src/test/ruby/shell/shell_test.rb
##########
@@ -123,14 +124,35 @@ class FooC; end
 
     # check that at least one of the HBase constants is present while evaluating
     io = StringIO.new <<~EOF
-      ROWPREFIXFILTER
+      ROWPREFIXFILTER.dump
     EOF
     output = capture_stdout { @shell.eval_io(io) }
     assert_match(/"ROWPREFIXFILTER"/, output)
   end
 
   #-----------------------------------------------------------------------------
 
+  define_test 'Shell::Shell#exception_handler should hide traceback' do
+    class TestException < RuntimeError; end
+    hide_traceback = true

Review comment:
       why do you both define a variable `hide_traceback = true` and also pass `true` to the `exception_handler` -- it's redundant?

##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -320,10 +323,20 @@ def eval_io(io)
       scanner = RubyLex.new
       scanner.set_input(io)
 
+      scanner.each_top_level_statement do |statement, linenum|
+        puts(workspace.evaluate(nil, statement, filename, linenum))

Review comment:
       this propagates source files and line numbers for the purpose of error handling? nice!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org