Merge pull request #21006 from Homebrew/progress-fixed-size-format

Use decimal prefixes for bytes, fix concurrent download progress output edge cases

Author: MikeMcQuaid

Library/Homebrew/download_queue.rb

@@ -259,19 +259,21 @@ def message_with_progress(downloadable, future, message)
       tty_width = Tty.width
       return message unless tty_width.positive?
 
+      available_width = tty_width - 2
       fetched_size = downloadable.fetched_size
-      return message if fetched_size.blank?
+      return message[0, available_width].to_s if fetched_size.blank?
 
+      precision = 1
       size_length = 5
       unit_length = 2
-      size_formatting_string = "%<size>#{size_length}.1f%<unit>#{unit_length}s"
-      size, unit = disk_usage_readable_size_unit(fetched_size)
+      size_formatting_string = "%<size>#{size_length}.#{precision}f%<unit>#{unit_length}s"
+      size, unit = disk_usage_readable_size_unit(fetched_size, precision:)
       formatted_fetched_size = format(size_formatting_string, size:, unit:)
 
       formatted_total_size = if future.fulfilled?
         formatted_fetched_size
       elsif (total_size = downloadable.total_size)
-        size, unit = disk_usage_readable_size_unit(total_size)
+        size, unit = disk_usage_readable_size_unit(total_size, precision:)
         format(size_formatting_string, size:, unit:)
       else
         # fill in the missing spaces for the size if we don't have it yet.
@@ -280,11 +282,11 @@ def message_with_progress(downloadable, future, message)
 
       max_phase_length = 11
       phase = format("%-<phase>#{max_phase_length}s", phase: downloadable.phase.to_s.capitalize)
-      progress = "#{formatted_fetched_size}/#{formatted_total_size}"
-      additional_padding_length = max_phase_length + size_length + unit_length - 1
-      message_length = tty_width - progress.length - additional_padding_length
+      progress = " [#{phase} #{formatted_fetched_size}/#{formatted_total_size}]"
+      message_length = available_width - progress.length
+      return message[0, available_width].to_s unless message_length.positive?
 
-      "#{message[0, message_length].to_s.ljust(message_length)} [#{phase} #{progress}]"
+      "#{message[0, message_length].to_s.ljust(message_length)}#{progress}"
     end
 
     class Spinner

Library/Homebrew/extend/kernel.rb

@@ -204,20 +204,20 @@ def ensure_executable!(name, formula_name = nil, reason: "", latest: false)
     Formulary.factory_stub(formula_name).ensure_installed!(reason:, latest:).opt_bin/name
   end
 
-  sig { params(size_in_bytes: T.any(Integer, Float)).returns([T.any(Integer, Float), String]) }
-  def disk_usage_readable_size_unit(size_in_bytes)
-    if size_in_bytes.abs >= 1_073_741_824
-      size = size_in_bytes.to_f / 1_073_741_824
-      unit = "GB"
-    elsif size_in_bytes.abs >= 1_048_576
-      size = size_in_bytes.to_f / 1_048_576
-      unit = "MB"
-    elsif size_in_bytes.abs >= 1_024
-      size = size_in_bytes.to_f / 1_024
-      unit = "KB"
-    else
-      size = size_in_bytes
-      unit = "B"
+  sig {
+    params(
+      size_in_bytes: T.any(Integer, Float),
+      precision:     T.nilable(Integer),
+    ).returns([T.any(Integer, Float), String])
+  }
+  def disk_usage_readable_size_unit(size_in_bytes, precision: nil)
+    size = size_in_bytes
+    unit = "B"
+    %w[KB MB GB].each do |next_unit|
+      break if (precision ? size.abs.round(precision) : size.abs) < 1000
+
+      size /= 1000.0
+      unit = next_unit
     end
     [size, unit]
   end

Library/Homebrew/test/extend/kernel_spec.rb

@@ -71,11 +71,11 @@
 
   specify "#disk_usage_readable" do
     expect(disk_usage_readable(1)).to eq("1B")
-    expect(disk_usage_readable(1000)).to eq("1000B")
-    expect(disk_usage_readable(1024)).to eq("1KB")
+    expect(disk_usage_readable(999)).to eq("999B")
+    expect(disk_usage_readable(1000)).to eq("1KB")
     expect(disk_usage_readable(1025)).to eq("1KB")
-    expect(disk_usage_readable(4_404_020)).to eq("4.2MB")
-    expect(disk_usage_readable(4_509_715_660)).to eq("4.2GB")
+    expect(disk_usage_readable(4_404_020)).to eq("4.4MB")
+    expect(disk_usage_readable(4_509_715_660)).to eq("4.5GB")
   end
 
   describe "#number_readable" do