Update `knife config list` with new behavior and fix tests.

Signed-off-by: Pete Higgins <pete@peterhiggins.org>

3 files changed, 54 insertions, 46 deletions

diff --git a/lib/chef/knife/config_list.rb b/lib/chef/knife/config_list.rb
index 9646178289..0a5d493273 100644
--- a/lib/chef/knife/config_list.rb
+++ b/lib/chef/knife/config_list.rb
@@ -1,6 +1,5 @@
 #
-# Author:: Vivek Singh (<vsingh@chef.io>)
-# Copyright:: Copyright (c) Chef Software Inc.
+# Copyright:: Copyright (c) 2018, Noah Kantrowitz
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -22,9 +21,14 @@ class Chef
   class Knife
     class ConfigList < Knife
       banner "knife config list (options)"
+      category "deprecated"
+
+      TABLE_HEADER = [" Profile", "Client", "Key", "Server"].freeze
 
       deps do
         require_relative "../workstation_config_loader"
+        require "tty-screen" unless defined?(TTY::Screen)
+        require "tty-table" unless defined?(TTY::Table)
       end
 
       option :ignore_knife_rb,
@@ -91,36 +95,45 @@ class Chef
 
       private
 
-      def render_table(profiles, padding: 2)
-        # Replace the home dir in the client key path with ~.
+      def render_table(profiles, padding: 1)
+        rows = []
+        # Render the data to a 2D array that will be used for the table.
         profiles.each do |profile|
+          # Replace the home dir in the client key path with ~.
           profile[:client_key] = profile[:client_key].to_s.gsub(/^#{Regexp.escape(Dir.home)}/, "~") if profile[:client_key]
+          profile[:profile] = "#{profile[:active] ? "*" : " "}#{profile[:profile]}"
+          rows << profile.values_at(:profile, :client_name, :client_key, :server_url)
         end
-        # Render the data to a 2D array that will be used for the table.
-        table_data = [["", "Profile", "Client", "Key", "Server"]] + profiles.map do |profile|
-          [profile[:active] ? "*" : ""] + profile.values_at(:profile, :client_name, :client_key, :server_url).map(&:to_s)
-        end
-        # Compute column widths.
-        column_widths = Array.new(table_data.first.length) do |i|
-          table_data.map { |row| row[i].length + padding }.max
-        end
-        # Special case, the first col gets no padding (because indicator) and last
-        # get no padding because last.
-        column_widths[0] -= padding
-        column_widths[-1] -= padding
-        # Build the format string for each row.
-        format_string = column_widths.map { |w| "%-#{w}.#{w}s" }.join("")
-        format_string << "\n"
-        # Print the header row and a separator.
-        table = ui.color(format_string % table_data.first, :green)
-        table << "-" * column_widths.sum
-        table << "\n"
-        # Print the rest of the table.
-        table_data.drop(1).each do |row|
-          table << format_string % row
+
+        table = TTY::Table.new(header: TABLE_HEADER, rows: rows)
+
+        # Rotate the table to vertical if the screen width is less than table width.
+        if table.width > TTY::Screen.width
+          table.orientation = :vertical
+          table.rotate
+          # Add a new line after each profile record.
+          table.render do |renderer|
+            renderer.border do
+              separator ->(row) { (row + 1) % TABLE_HEADER.size == 0 }
+            end
+            # Remove the leading space added of the first column.
+            renderer.filter = Proc.new do |val, row_index, col_index|
+              if col_index == 1 || (row_index) % TABLE_HEADER.size == 0
+                val.strip
+              else
+                val
+              end
+            end
+          end
+        else
+          table.render do |renderer|
+            renderer.border do
+              mid   "-"
+              style :green
+            end
+            renderer.padding = [0, padding, 0, 0] # pad right with 2 characters
+          end
         end
-        # Trim the last newline because ui.output adds one.
-        table.chomp!
       end
 
     end
diff --git a/spec/integration/knife/config_list_profiles_spec.rb b/spec/integration/knife/config_list_spec.rb
index 08372e646a..03409e4983 100644
--- a/spec/integration/knife/config_list_profiles_spec.rb
+++ b/spec/integration/knife/config_list_spec.rb
@@ -17,7 +17,7 @@ require "spec_helper"
 require "support/shared/integration/integration_helper"
 require "support/shared/context/config"
 
-describe "knife config list-profiles", :workstation do
+describe "knife config list", :workstation do
   include IntegrationSupport
   include KnifeSupport
 
@@ -25,14 +25,14 @@ describe "knife config list-profiles", :workstation do
 
   when_the_repository("has a custom env") do
     let(:cmd_args) { [] }
-    let(:knife_list_profiles) do
-      knife("config", "list-profiles", *cmd_args, instance_filter: lambda { |instance|
+    let(:knife_list) do
+      knife("config", "list", *cmd_args, instance_filter: lambda { |instance|
         # Fake the failsafe check because this command doesn't actually process knife.rb.
         $__KNIFE_INTEGRATION_FAILSAFE_CHECK << " ole"
         allow(File).to receive(:file?).and_call_original
       })
     end
-    subject { knife_list_profiles.stdout }
+    subject { knife_list.stdout }
 
     around do |ex|
       # Store and reset the value of some env vars.
@@ -65,13 +65,13 @@ describe "knife config list-profiles", :workstation do
     # substantial trailing whitespace in most cases which many editors "helpfully" remove.
 
     context "with no credentials file" do
-      subject { knife_list_profiles.stderr }
+      subject { knife_list.stderr }
       it { is_expected.to eq "FATAL: No profiles found, #{path_to(".chef/credentials")} does not exist or is empty\n" }
     end
 
     context "with an empty credentials file" do
       before { file(".chef/credentials", "") }
-      subject { knife_list_profiles.stderr }
+      subject { knife_list.stderr }
       it { is_expected.to eq "FATAL: No profiles found, #{path_to(".chef/credentials")} does not exist or is empty\n" }
     end
 
diff --git a/spec/integration/knife/config_use_profile_spec.rb b/spec/integration/knife/config_use_spec.rb
index 6c4c9e92eb..41d575669c 100644
--- a/spec/integration/knife/config_use_profile_spec.rb
+++ b/spec/integration/knife/config_use_spec.rb
@@ -17,7 +17,7 @@ require "spec_helper"
 require "support/shared/integration/integration_helper"
 require "support/shared/context/config"
 
-describe "knife config use-profile", :workstation do
+describe "knife config use", :workstation do
   include IntegrationSupport
   include KnifeSupport
 
@@ -26,15 +26,15 @@ describe "knife config use-profile", :workstation do
   let(:cmd_args) { [] }
 
   when_the_repository("has a custom env") do
-    let(:knife_use_profile) do
-      knife("config", "use-profile", *cmd_args, instance_filter: lambda { |instance|
+    let(:knife_use) do
+      knife("config", "use", *cmd_args, instance_filter: lambda { |instance|
         # Fake the failsafe check because this command doesn't actually process knife.rb.
         $__KNIFE_INTEGRATION_FAILSAFE_CHECK << " ole"
         allow(File).to receive(:file?).and_call_original
       })
     end
 
-    subject { knife_use_profile.stdout }
+    subject { knife_use.stdout }
 
     around do |ex|
       # Store and reset the value of some env vars.
@@ -67,11 +67,6 @@ describe "knife config use-profile", :workstation do
       ENV["HOME"] = path_to(".")
     end
 
-    context "with no argument" do
-      subject { knife_use_profile.stderr }
-      it { is_expected.to eq "FATAL: You must specify a profile\n" }
-    end
-
     context "with an argument" do
       let(:cmd_args) { %w{production} }
       before { file(".chef/credentials", <<~EOH) }
@@ -88,14 +83,14 @@ describe "knife config use-profile", :workstation do
 
     context "with no credentials file" do
       let(:cmd_args) { %w{production} }
-      subject { knife_use_profile.stderr }
+      subject { knife_use.stderr }
       it { is_expected.to eq "FATAL: No profiles found, #{path_to(".chef/credentials")} does not exist or is empty\n" }
     end
 
     context "with an empty credentials file" do
       let(:cmd_args) { %w{production} }
       before { file(".chef/credentials", "") }
-      subject { knife_use_profile.stderr }
+      subject { knife_use.stderr }
       it { is_expected.to eq "FATAL: No profiles found, #{path_to(".chef/credentials")} does not exist or is empty\n" }
     end
 
@@ -107,7 +102,7 @@ describe "knife config use-profile", :workstation do
         client_key = "testkey.pem"
         chef_server_url = "https://example.com/organizations/testorg"
       EOH
-      subject { knife_use_profile }
+      subject { knife_use }
       it { expect { subject }.to raise_error ChefConfig::ConfigurationError, "Profile staging doesn't exist. Please add it to #{path_to(".chef/credentials")} and if it is profile with DNS name check that you are not missing single quotes around it as per docs https://docs.chef.io/workstation/knife_setup/#knife-profiles." }
     end