Non-zero exit status for bad scripts.

When gitsh is invoked in a non-interactive manner, a parse error should
result in a non-zero exit status.

To make sure this happens, parse error handling has been delegated to the
input strategies by the interpreter. In an interactive session, we output an
error message and continue the session. In a non-interactive session, we
raise a Gitsh::ParseError, which will bubble up to the gitsh binary and
terminate the session with an exit status of 1.

Since the input strategy for files is used in a few internal places (in the
interactive input strategy, for loading .gitshrc, and in the :source
command), those call sites have been modified to handle the possibility of a
Gitsh::ParseError.

lib/gitsh/commands/internal_command.rb

@@ -1,3 +1,5 @@
+require 'gitsh/error'
+
 module Gitsh::Commands
   module InternalCommand
     def self.new(command, args)
@@ -166,8 +168,7 @@ def self.help_message
 
       def execute(env)
         if valid_arguments?
-          Gitsh::FileRunner.run(env: env, path: path(env))
-          true
+          run_script(env)
         else
           env.puts_error USAGE_MESSAGE
           false
@@ -176,6 +177,14 @@ def execute(env)
 
       private
 
+      def run_script(env)
+        Gitsh::FileRunner.run(env: env, path: path(env))
+        true
+      rescue Gitsh::ParseError => e
+        env.puts_error("gitsh: #{e.message}")
+        false
+      end
+
       def valid_arguments?
         args.length == 1
       end

lib/gitsh/error.rb

@@ -7,4 +7,7 @@ class UnsetVariableError < Error
 
   class NoInputError < Error
   end
+
+  class ParseError < Error
+  end
 end

lib/gitsh/input_strategies/file.rb

@@ -30,6 +30,10 @@ def read_command
         nil
       end
 
+      def handle_parse_error(message)
+        raise ParseError, message
+      end
+
       private
 
       attr_reader :env, :file, :path

lib/gitsh/input_strategies/interactive.rb

@@ -48,6 +48,11 @@ def read_command
         retry
       end
 
+
+      def handle_parse_error(message)
+        env.puts_error("gitsh: #{message}")
+      end
+
       private
 
       attr_reader :history, :line_editor, :env, :terminal
@@ -83,6 +88,8 @@ def greeting_enabled?
 
       def load_gitshrc
         FileRunner.run(env: env, path: gitshrc_path)
+      rescue ParseError => e
+        env.puts_error "gitsh: #{e.message}"
       rescue NoInputError
       end
 

lib/gitsh/interpreter.rb

@@ -28,7 +28,7 @@ def run
     def execute(input)
       build_command(input).execute(env)
     rescue RLTK::LexingError, RLTK::NotInLanguage, RLTK::BadToken
-      env.puts_error('gitsh: parse error')
+      input_strategy.handle_parse_error('parse error')
     end
 
     def build_command(input)

spec/integration/error_handling_spec.rb

@@ -16,4 +16,14 @@
       expect(gitsh).to output_error /gitsh: parse error/
     end
   end
+
+  it 'does not explode when given a badly formatted script' do
+    in_a_temporary_directory do
+      write_file('bad.gitsh', ":echo 'foo")
+
+      expect("#{gitsh_path} bad.gitsh").
+        to execute.with_exit_status(1).
+        with_error_output_matching(/parse error/)
+    end
+  end
 end

spec/integration/running_scripts_spec.rb

@@ -6,15 +6,15 @@
       in_a_temporary_directory do
         write_file('myscript.gitsh', "init\n\ncommit")
 
-        expect("#{gitsh} --git #{fake_git_path} myscript.gitsh").to execute.
+        expect("#{gitsh_path} --git #{fake_git_path} myscript.gitsh").to execute.
           successfully.
           with_output_matching(/^Fake git: init\nFake git: commit\n$/)
       end
     end
 
     context 'when the script file does not exist' do
       it 'exits with a useful error message' do
-        expect("#{gitsh} --git #{fake_git_path} noscript.gitsh").to execute.
+        expect("#{gitsh_path} --git #{fake_git_path} noscript.gitsh").to execute.
           with_exit_status(66).
           with_error_output_matching(/^gitsh: noscript\.gitsh: No such file or directory$/)
       end
@@ -26,14 +26,10 @@
       in_a_temporary_directory do
         write_file('myscript.gitsh', "init\n\ncommit")
 
-        expect("cat myscript.gitsh | #{gitsh} --git #{fake_git_path}").
+        expect("cat myscript.gitsh | #{gitsh_path} --git #{fake_git_path}").
           to execute.successfully.
           with_output_matching(/^Fake git: init\nFake git: commit\n$/)
       end
     end
   end
-
-  def gitsh
-    File.expand_path('../../../bin/gitsh', __FILE__)
-  end
 end

spec/support/scripts.rb

@@ -0,0 +1,9 @@
+module Scripts
+  def gitsh_path
+    File.expand_path('../../../bin/gitsh', __FILE__)
+  end
+end
+
+RSpec.configure do |config|
+  config.include Scripts
+end

spec/units/commands/internal/source_spec.rb

@@ -30,5 +30,19 @@
         expect(result).to eq false
       end
     end
+
+    context 'with a file that fails to parse' do
+      it 'prints an error message and returns false' do
+        env = spy('env', puts_error: nil)
+        command = described_class.new('source', arguments('/bad_script'))
+        allow(Gitsh::FileRunner).
+          to receive(:run).and_raise(Gitsh::ParseError, 'Oh no!')
+
+        result = command.execute(env)
+
+        expect(env).to have_received(:puts_error).with('gitsh: Oh no!')
+        expect(result).to eq false
+      end
+    end
   end
 end

spec/units/input_strategies/file_spec.rb

@@ -79,4 +79,13 @@
       end
     end
   end
+
+  describe '#handle_parse_error' do
+    it 'raises' do
+      input_strategy = described_class.new(path: double)
+
+      expect { input_strategy.handle_parse_error('my message') }.
+        to raise_exception(Gitsh::ParseError, 'my message')
+    end
+  end
 end

spec/units/input_strategies/interactive_spec.rb

@@ -29,6 +29,16 @@
       expect(Gitsh::FileRunner).to have_received(:run).
         with(hash_including(path: "#{ENV['HOME']}/.gitshrc"))
     end
+
+    it 'handles parse errors in the ~/.gitshrc file' do
+      input_strategy = build_input_strategy
+      allow(Gitsh::FileRunner).
+        to receive(:run).and_raise(Gitsh::ParseError, 'my message')
+
+      input_strategy.setup
+
+      expect(env).to have_received(:puts_error).with('gitsh: my message')
+    end
   end
 
   describe '#teardown' do
@@ -97,6 +107,17 @@
     end
   end
 
+  describe '#handle_parse_error' do
+    it 'outputs the error' do
+      input_strategy = build_input_strategy
+      allow(env).to receive(:puts_error)
+
+      input_strategy.handle_parse_error('my message')
+
+      expect(env).to have_received(:puts_error).with('gitsh: my message')
+    end
+  end
+
   def build_input_strategy(options={})
     described_class.new(
       line_editor: options.fetch(:line_editor, line_editor),
@@ -126,6 +147,7 @@ def env
     @env ||= double('Environment', {
       print: nil,
       puts: nil,
+      puts_error: nil,
       repo_config_color: '',
       fetch: '',
       :[] => nil

spec/units/interpreter_spec.rb

@@ -38,7 +38,12 @@
       allow(parser).to receive(:parse).
         and_raise(RLTK::NotInLanguage.new([], double(:token), []))
       lexer = double('Lexer', lex: double(:tokens))
-      input_strategy = double(:input_strategy, setup: nil, teardown: nil)
+      input_strategy = double(
+        :input_strategy,
+        setup: nil,
+        teardown: nil,
+        handle_parse_error: nil,
+      )
       allow(input_strategy).to receive(:read_command).and_return(
         'bad command',
         nil,
@@ -51,7 +56,8 @@
 
       interpreter.run
 
-      expect(env).to have_received(:puts_error).with('gitsh: parse error')
+      expect(input_strategy).
+        to have_received(:handle_parse_error).with('parse error')
     end
   end
 end