Merge pull request #2857 from mitchellh/f-ui-revamp

UI revamp

bin/vagrant

@@ -124,7 +124,7 @@ begin
 
   if !Vagrant.in_installer?
     # If we're not in the installer, warn.
-    env.ui.warn(I18n.t("vagrant.general.not_in_installer") + "\n")
+    env.ui.warn(I18n.t("vagrant.general.not_in_installer") + "\n", prefix: false)
   end
 
   begin

lib/vagrant/action/builtin/wait_for_communicator.rb

@@ -50,7 +50,7 @@ module Vagrant
           end
 
           # Wait for a result or an interrupt
-          env[:ui].info I18n.t("vagrant.boot_waiting")
+          env[:ui].output(I18n.t("vagrant.boot_waiting"))
           while ready_thr.alive? && states_thr.alive?
             sleep 1
             return if env[:interrupted]
@@ -72,7 +72,7 @@ module Vagrant
             raise Errors::VMBootTimeout
           end
 
-          env[:ui].info I18n.t("vagrant.boot_completed")
+          env[:ui].output(I18n.t("vagrant.boot_completed"))
 
           # Make sure our threads are all killed
           ready_thr.kill

lib/vagrant/plugin/v2/command.rb

@@ -188,7 +188,14 @@ module Vagrant
           machines.reverse! if options[:reverse]
 
           # Go through each VM and yield it!
+          color_order = [:white]
+          color_index = 0
+
           machines.each do |machine|
+            # Set the machine color
+            machine.ui.opts[:color] = color_order[color_index % color_order.length]
+            color_index += 1
+
             @logger.info("With machine: #{machine.name} (#{machine.provider.inspect})")
             yield machine
           end

lib/vagrant/plugin/v2/communicator.rb

@@ -49,6 +49,10 @@ module Vagrant
         # wait_for_ready waits until the communicator is ready, blocking
         # until then. It will wait up to the given duration or raise an
         # exception if something goes wrong.
+        #
+        # @param [Fixnum] duration Timeout in seconds.
+        # @return [Boolean] Will return true on successful connection
+        #   or false on timeout.
         def wait_for_ready(duration)
           # By default, we implement a naive solution.
           begin

lib/vagrant/ui.rb

@@ -15,11 +15,16 @@ module Vagrant
     # * `error`
     # * `success`
     class Interface
+      # Opts can be used to set some options. These options are implementation
+      # specific. See the implementation for more docs.
+      attr_accessor :opts
+
       def initialize
         @logger   = Log4r::Logger.new("vagrant::ui::interface")
+        @opts     = {}
       end
 
-      [:ask, :warn, :error, :info, :success].each do |method|
+      [:ask, :detail, :warn, :error, :info, :output, :success].each do |method|
         define_method(method) do |message, *opts|
           # Log normal console messages
           @logger.info { "#{method}: #{message}" }
@@ -105,6 +110,9 @@ module Vagrant
     class Basic < Interface
       include Util::SafePuts
 
+      # The prefix for `output` messages.
+      OUTPUT_PREFIX = "==> "
+
       def initialize
         super
 
@@ -114,7 +122,7 @@ module Vagrant
       # Use some light meta-programming to create the various methods to
       # output text to the UI. These all delegate the real functionality
       # to `say`.
-      [:info, :warn, :error, :success].each do |method|
+      [:detail, :info, :warn, :error, :output, :success].each do |method|
         class_eval <<-CODE
           def #{method}(message, *args)
             super(message)
@@ -168,9 +176,9 @@ module Vagrant
 
       # This method handles actually outputting a message of a given type
       # to the console.
-      def say(type, message, opts=nil)
+      def say(type, message, **opts)
         defaults = { :new_line => true, :prefix => true }
-        opts     = defaults.merge(opts || {})
+        opts     = defaults.merge(@opts).merge(opts)
 
         # Determine whether we're expecting to output our
         # own new line or not.
@@ -197,15 +205,25 @@ module Vagrant
       end
 
       # This is called by `say` to format the message for output.
-      def format_message(type, message, opts=nil)
+      def format_message(type, message, **opts)
-        opts ||= {}
+        prefix = ""
-        message = "[#{opts[:scope]}] #{message}" if opts[:scope] && opts[:prefix]
+        if !opts.has_key?(:prefix) || opts[:prefix]
-        message
+          prefix = OUTPUT_PREFIX
+          prefix = " " * OUTPUT_PREFIX.length if type == :detail
+        end
+
+        # Fast-path if there is no prefix
+        return message if prefix.empty?
+
+        # Otherwise, make sure to prefix every line properly
+        message.split("\n").map { |line| "#{prefix}#{line}" }.join("\n")
       end
     end
 
     # This implements a scope for the {Basic} UI.
     class BasicScope < Interface
+      attr_reader :scope, :ui
+
       def initialize(ui, scope)
         super()
 
@@ -213,10 +231,23 @@ module Vagrant
         @scope = scope
       end
 
-      [:ask, :warn, :error, :info, :success].each do |method|
+      # Return the parent's opts.
+      #
+      # @return [Hash]
+      def opts
+        @ui.opts
+      end
+
+      [:ask, :detail, :warn, :error, :info, :output, :success].each do |method|
         define_method(method) do |message, opts=nil|
           opts ||= {}
           opts[:scope] = @scope
+          if !opts.has_key?(:prefix) || opts[:prefix]
+            prefix = "#{@scope}: "
+            message = message.split("\n").map do |line|
+              "#{prefix}#{line}"
+            end.join("\n")
+          end
           @ui.send(method, message, opts)
         end
       end
@@ -241,34 +272,36 @@ module Vagrant
     class Colored < Basic
       # Terminal colors
       COLORS = {
-        :clear  => "\e[0m",
+        red:     31,
-        :red    => "\e[31m",
+        green:   32,
-        :green  => "\e[32m",
+        yellow:  33,
-        :yellow => "\e[33m"
+        blue:    34,
-      }
+        magenta: 35,
-
+        cyan:    36,
-      # Mapping between type of message and the color to output
+        white:   37,
-      COLOR_MAP = {
-        :warn    => COLORS[:yellow],
-        :error   => COLORS[:red],
-        :success => COLORS[:green]
       }
 
       # This is called by `say` to format the message for output.
-      def format_message(type, message, opts=nil)
+      def format_message(type, message, **opts)
         # Get the format of the message before adding color.
         message = super
 
-        # Colorize the message if there is a color for this type of message,
+        opts = @opts.merge(opts)
-        # either specified by the options or via the default color map.
-        if opts.has_key?(:color)
-          color   = COLORS[opts[:color]]
-          message = "#{color}#{message}#{COLORS[:clear]}"
-        else
-          message = "#{COLOR_MAP[type]}#{message}#{COLORS[:clear]}" if COLOR_MAP[type]
-        end
 
-        message
+        # Special case some colors for certain message types
+        opts[:color] = :red if type == :error
+        opts[:color] = :yellow if type == :warn
+
+        # If there is no color specified, exit early
+        return message if !opts.has_key?(:color)
+
+        # If it is a detail, it is not bold. Every other message type
+        # is bolded.
+        bold  = type != :detail
+        color = COLORS[opts[:color]]
+
+        # Color the message and make sure to reset the color at the end
+        "\033[#{bold ? 1 : 0};#{color}m#{message}\033[0m"
       end
     end
   end

lib/vagrant/util/platform.rb

@@ -110,6 +110,7 @@ module Vagrant
           if windows?
             return true if ENV.has_key?("ANSICON")
             return true if cygwin?
+            return true if ENV["TERM"] == "cygwin"
             return false
           end
 

plugins/providers/virtualbox/action/check_guest_additions.rb

@@ -7,11 +7,13 @@ module VagrantPlugins
         end
 
         def call(env)
+          env[:ui].output(I18n.t("vagrant.virtualbox.checking_guest_additions"))
+
           # Use the raw interface for now, while the virtualbox gem
           # doesn't support guest properties (due to cross platform issues)
           version = env[:machine].provider.driver.read_guest_additions_version
           if !version
-            env[:ui].warn I18n.t("vagrant.actions.vm.check_guest_additions.not_detected")
+            env[:ui].detail(I18n.t("vagrant.actions.vm.check_guest_additions.not_detected"))
           else
             # Read the versions
             versions = [version, env[:machine].provider.driver.version]
@@ -29,7 +31,7 @@ module VagrantPlugins
             vb_version    = versions[1]
 
             if guest_version != vb_version
-              env[:ui].warn(I18n.t("vagrant.actions.vm.check_guest_additions.version_mismatch",
+              env[:ui].detail(I18n.t("vagrant.actions.vm.check_guest_additions.version_mismatch",
                                    :guest_version => version,
                                    :virtualbox_version => vb_version))
             end

plugins/providers/virtualbox/action/forward_ports.rb

@@ -25,7 +25,7 @@ module VagrantPlugins
             end
           end
 
-          env[:ui].info I18n.t("vagrant.actions.vm.forward_ports.forwarding")
+          env[:ui].output(I18n.t("vagrant.actions.vm.forward_ports.forwarding"))
           forward_ports
 
           @app.call(env)
@@ -47,7 +47,7 @@ module VagrantPlugins
             # because the VM is using Virtualbox NAT networking. Host-only
             # bridged networking don't require port-forwarding and establishing
             # forwarded ports on these attachment types has uncertain behaviour.
-            @env[:ui].info(I18n.t("vagrant.actions.vm.forward_ports.forwarding_entry",
+            @env[:ui].detail(I18n.t("vagrant.actions.vm.forward_ports.forwarding_entry",
                                     message_attributes))
 
             # Verify we have the network interface to attach to
@@ -61,7 +61,7 @@ module VagrantPlugins
             # Port forwarding requires the network interface to be a NAT interface,
             # so verify that that is the case.
             if interfaces[fp.adapter][:type] != :nat
-              @env[:ui].info(I18n.t("vagrant.actions.vm.forward_ports.non_nat",
+              @env[:ui].detail(I18n.t("vagrant.actions.vm.forward_ports.non_nat",
                                     message_attributes))
               next
             end

plugins/providers/virtualbox/action/network.rb

@@ -104,7 +104,16 @@ module VagrantPlugins
           if !adapters.empty?
             # Enable the adapters
             @logger.info("Enabling adapters...")
-            env[:ui].info I18n.t("vagrant.actions.vm.network.preparing")
+            env[:ui].output(I18n.t("vagrant.actions.vm.network.preparing"))
+            adapters.each do |adapter|
+              env[:ui].detail(I18n.t(
+                "vagrant.virtualbox.network_adapter",
+                adapter: adapter[:adapter].to_s,
+                type: adapter[:type].to_s,
+                extra: "",
+              ))
+            end
+
             env[:machine].provider.driver.enable_adapters(adapters)
           end
 

plugins/providers/virtualbox/action/set_name.rb

@@ -35,8 +35,8 @@ module VagrantPlugins
           if vms.has_key?(name)
             @logger.info("Not setting the name because our name is already set.")
           else
-            @logger.info("Setting the name of the VM: #{name}")
+            env[:ui].info(I18n.t(
-            env[:ui].info(I18n.t("vagrant.actions.vm.set_name.setting_name"))
+              "vagrant.actions.vm.set_name.setting_name", name: name))
             env[:machine].provider.driver.set_name(name)
           end
 

plugins/providers/virtualbox/synced_folder.rb

@@ -35,12 +35,13 @@ module VagrantPlugins
         end
 
         # Go through each folder and mount
-        machine.ui.info(I18n.t("vagrant.actions.vm.share_folders.mounting"))
+        machine.ui.output(I18n.t("vagrant.actions.vm.share_folders.mounting"))
         folders.each do |id, data|
           if data[:guestpath]
             # Guest path specified, so mount the folder to specified point
-            machine.ui.info(I18n.t("vagrant.actions.vm.share_folders.mounting_entry",
+            machine.ui.detail(I18n.t("vagrant.actions.vm.share_folders.mounting_entry",
-                                  :guest_path => data[:guestpath]))
+                                  guestpath: data[:guestpath],
+                                  hostpath: data[:hostpath]))
 
             # Dup the data so we can pass it to the guest API
             data = data.dup
@@ -55,8 +56,8 @@ module VagrantPlugins
               :mount_virtualbox_shared_folder, id, data[:guestpath], data)
           else
             # If no guest path is specified, then automounting is disabled
-            machine.ui.info(I18n.t("vagrant.actions.vm.share_folders.nomount_entry",
+            machine.ui.detail(I18n.t("vagrant.actions.vm.share_folders.nomount_entry",
-                                  :host_path => data[:hostpath]))
+                                  :hostpath => data[:hostpath]))
           end
         end
       end

templates/locales/en.yml

@@ -124,6 +124,10 @@ en:
         The 'run_file' specified could not be found.
 
     virtualbox:
+      checking_guest_additions: |-
+        Checking for guest additions in VM...
+      network_adapter: |-
+        Adapter %{adapter}: %{type}%{extra}
       config:
         id_in_pre_import: |-
           The ':id' parameter is not available in "pre-import" customizations.
@@ -1114,7 +1118,7 @@ en:
             Fixed port collision for %{guest_port} => %{host_port}. Now on port %{new_port}.
           forwarding: Forwarding ports...
           forwarding_entry: |-
-            -- %{guest_port} => %{host_port} (adapter %{adapter})
+            %{guest_port} => %{host_port} (adapter %{adapter})
           non_nat: |-
             VirtualBox adapter #%{adapter} not configured as "NAT". Skipping port
             forwards on this adapter.
@@ -1185,11 +1189,11 @@ en:
         share_folders:
           creating: Creating shared folders metadata...
           mounting: Mounting shared folders...
-          mounting_entry: "-- %{guest_path}"
+          mounting_entry: "%{guestpath} => %{hostpath}"
-          nomount_entry: "-- Automounting disabled: %{host_path}"
+          nomount_entry: "Automounting disabled: %{hostpath}"
         set_name:
           setting_name: |-
-            Setting the name of the VM...
+            Setting the name of the VM: %{name}
         suspend:
           suspending: Saving VM state and suspending execution...
 

test/unit/vagrant/plugin/v2/command_test.rb

@@ -75,9 +75,11 @@ describe Vagrant::Plugin::V2::Command do
     it "should yield every VM in order is no name is given" do
       foo_vm = double("foo")
       foo_vm.stub(:name => "foo", :provider => :foobarbaz)
+      foo_vm.stub(ui: Vagrant::UI::Silent.new)
 
       bar_vm = double("bar")
       bar_vm.stub(:name => "bar", :provider => :foobarbaz)
+      bar_vm.stub(ui: Vagrant::UI::Silent.new)
 
       environment.stub(:machine_names => [:foo, :bar])
       environment.stub(:machine).with(:foo, default_provider).and_return(foo_vm)
@@ -102,6 +104,7 @@ describe Vagrant::Plugin::V2::Command do
     it "yields the given VM if a name is given" do
       foo_vm = double("foo")
       foo_vm.stub(:name => "foo", :provider => :foobarbaz)
+      foo_vm.stub(ui: Vagrant::UI::Silent.new)
 
       environment.stub(:machine).with(:foo, default_provider).and_return(foo_vm)
 
@@ -115,6 +118,7 @@ describe Vagrant::Plugin::V2::Command do
       provider = :foobarbaz
 
       foo_vm.stub(:name => "foo", :provider => provider)
+      foo_vm.stub(ui: Vagrant::UI::Silent.new)
       environment.stub(:machine).with(:foo, provider).and_return(foo_vm)
 
       vms = []
@@ -138,6 +142,7 @@ describe Vagrant::Plugin::V2::Command do
       environment.stub(:active_machines => [[name, provider]])
       environment.stub(:machine).with(name, provider).and_return(vmware_vm)
       vmware_vm.stub(:name => name, :provider => provider)
+      vmware_vm.stub(ui: Vagrant::UI::Silent.new)
 
       vms = []
       instance.with_target_vms(name.to_s) { |vm| vms << vm }
@@ -151,7 +156,7 @@ describe Vagrant::Plugin::V2::Command do
 
       environment.stub(:active_machines => [[name, provider]])
       environment.stub(:machine).with(name, provider).and_return(vmware_vm)
-      vmware_vm.stub(:name => name, :provider => provider)
+      vmware_vm.stub(:name => name, :provider => provider, ui: Vagrant::UI::Silent.new)
 
       vms = []
       instance.with_target_vms(name.to_s, :provider => provider) { |vm| vms << vm }
@@ -164,6 +169,7 @@ describe Vagrant::Plugin::V2::Command do
 
       environment.stub(:machine).with(name, default_provider).and_return(machine)
       machine.stub(:name => name, :provider => default_provider)
+      machine.stub(ui: Vagrant::UI::Silent.new)
 
       results = []
       instance.with_target_vms(name.to_s) { |m| results << m }
@@ -180,6 +186,7 @@ describe Vagrant::Plugin::V2::Command do
       environment.stub(:machine_names => [])
       environment.stub(:primary_machine_name => name)
       vmware_vm.stub(:name => name, :provider => provider)
+      vmware_vm.stub(ui: Vagrant::UI::Silent.new)
 
       vms = []
       instance.with_target_vms(nil, :single_target => true) { |vm| vms << vm }
@@ -195,6 +202,7 @@ describe Vagrant::Plugin::V2::Command do
       environment.stub(:machine_names => [])
       environment.stub(:primary_machine_name => name)
       machine.stub(:name => name, :provider => default_provider)
+      machine.stub(ui: Vagrant::UI::Silent.new)
 
       vms = []
       instance.with_target_vms(nil, :single_target => true) { |vm| vms << machine }

test/unit/vagrant/ui_test.rb

@@ -0,0 +1,228 @@
+require File.expand_path("../../base", __FILE__)
+
+describe Vagrant::UI::Basic do
+  context "in general" do
+    it "outputs within the a new thread" do
+      current = Thread.current.object_id
+
+      subject.should_receive(:safe_puts).with do |*args|
+        expect(Thread.current.object_id).to_not eq(current)
+        true
+      end
+
+      subject.output("foo")
+    end
+
+    it "outputs using `puts` by default" do
+      subject.should_receive(:safe_puts).with do |message, **opts|
+        expect(opts[:printer]).to eq(:puts)
+        true
+      end
+
+      subject.output("foo")
+    end
+
+    it "outputs using `print` if new_line is false" do
+      subject.should_receive(:safe_puts).with do |message, **opts|
+        expect(opts[:printer]).to eq(:print)
+        true
+      end
+
+      subject.output("foo", new_line: false)
+    end
+
+    it "outputs using `print` if new_line is false" do
+      subject.should_receive(:safe_puts).with do |message, **opts|
+        expect(opts[:printer]).to eq(:print)
+        true
+      end
+
+      subject.output("foo", new_line: false)
+    end
+
+    it "outputs to stdout" do
+      subject.should_receive(:safe_puts).with do |message, **opts|
+        expect(opts[:io]).to be($stdout)
+        true
+      end
+
+      subject.output("foo")
+    end
+
+    it "outputs to stderr for errors" do
+      subject.should_receive(:safe_puts).with do |message, **opts|
+        expect(opts[:io]).to be($stderr)
+        true
+      end
+
+      subject.error("foo")
+    end
+  end
+
+  describe "#detail" do
+    it "prefixes with spaces" do
+      subject.should_receive(:safe_puts).with("    foo", anything)
+      subject.detail("foo")
+    end
+
+    it "doesn't prefix if told not to" do
+      subject.should_receive(:safe_puts).with("foo", anything)
+      subject.detail("foo", prefix: false)
+    end
+
+    it "prefixes every line" do
+      subject.should_receive(:safe_puts).with("    foo\n    bar", anything)
+      subject.detail("foo\nbar")
+    end
+  end
+
+  describe "#output" do
+    it "prefixes with ==>" do
+      subject.should_receive(:safe_puts).with("==> foo", anything)
+      subject.output("foo")
+    end
+
+    it "doesn't prefix if told not to" do
+      subject.should_receive(:safe_puts).with("foo", anything)
+      subject.output("foo", prefix: false)
+    end
+
+    it "prefixes every line" do
+      subject.should_receive(:safe_puts).with("==> foo\n==> bar", anything)
+      subject.output("foo\nbar")
+    end
+  end
+
+  describe "#scope" do
+    it "creates a basic scope" do
+      scope = subject.scope("foo")
+      expect(scope.scope).to eql("foo")
+      expect(scope.ui).to be(subject)
+    end
+  end
+end
+
+describe Vagrant::UI::Colored do
+  include_context "unit"
+
+  before do
+    # We don't want any prefixes on anything...
+    subject.opts[:prefix] = false
+  end
+
+  describe "#detail" do
+    it "colors output nothing by default" do
+      subject.should_receive(:safe_puts).with("foo", anything)
+      subject.detail("foo")
+    end
+
+    it "does not bold by default with a color" do
+      subject.should_receive(:safe_puts).with do |message, *args|
+        expect(message).to start_with("\033[0;31m")
+        expect(message).to end_with("\033[0m")
+      end
+
+      subject.detail("foo", color: :red)
+    end
+  end
+
+  describe "#error" do
+    it "colors red" do
+      subject.should_receive(:safe_puts).with do |message, *args|
+        expect(message).to start_with("\033[1;31m")
+        expect(message).to end_with("\033[0m")
+      end
+
+      subject.error("foo")
+    end
+  end
+
+  describe "#output" do
+    it "colors output nothing by default" do
+      subject.should_receive(:safe_puts).with("foo", anything)
+      subject.output("foo")
+    end
+
+    it "colors output to color specified in global opts" do
+      subject.opts[:color] = :red
+
+      subject.should_receive(:safe_puts).with do |message, *args|
+        expect(message).to start_with("\033[1;31m")
+        expect(message).to end_with("\033[0m")
+      end
+
+      subject.output("foo")
+    end
+
+    it "colors output to specified color over global opts" do
+      subject.opts[:color] = :red
+
+      subject.should_receive(:safe_puts).with do |message, *args|
+        expect(message).to start_with("\033[1;32m")
+        expect(message).to end_with("\033[0m")
+      end
+
+      subject.output("foo", color: :green)
+    end
+  end
+
+  describe "#warn" do
+    it "colors yellow" do
+      subject.should_receive(:safe_puts).with do |message, *args|
+        expect(message).to start_with("\033[1;33m")
+        expect(message).to end_with("\033[0m")
+      end
+
+      subject.warn("foo")
+    end
+  end
+end
+
+describe Vagrant::UI::BasicScope do
+  let(:scope) { "foo" }
+  let(:ui)    { double("ui") }
+
+  subject { described_class.new(ui, scope) }
+
+  describe "#machine" do
+    it "sets the scope option" do
+      ui.should_receive(:machine).with(:foo, scope: scope)
+      subject.machine(:foo)
+    end
+
+    it "preserves existing options" do
+      ui.should_receive(:machine).with(:foo, :bar, foo: :bar, scope: scope)
+      subject.machine(:foo, :bar, foo: :bar)
+    end
+  end
+
+  describe "#opts" do
+    it "is the parent's opts" do
+      ui.stub(opts: Object.new)
+      expect(subject.opts).to be(ui.opts)
+    end
+  end
+
+  describe "#output" do
+    it "prefixes with the scope" do
+      ui.should_receive(:output).with("#{scope}: foo", anything)
+      subject.output("foo")
+    end
+
+    it "does not prefix if told not to" do
+      ui.should_receive(:output).with("foo", anything)
+      subject.output("foo", prefix: false)
+    end
+
+    it "prefixes every line" do
+      ui.should_receive(:output).with(
+        "#{scope}: foo\n#{scope}: bar", anything)
+      subject.output("foo\nbar")
+    end
+
+    it "puts the scope into the options hash" do
+      ui.should_receive(:output).with(anything, scope: scope)
+      subject.output("foo")
+    end
+  end
+end