dominikh / filesize

filesize is a small ruby class for handling filesizes with both the SI and binary prefixes, allowing conversion from any size to any other size.
MIT License
81 stars 16 forks source link

Parsing negative filesizes #18

Open drusepth opened 7 years ago

drusepth commented 7 years ago

Is there a reason this gem doesn't parse negative filesizes (e.g. -50KB)? Seems like when you're talking about memory capacities, it'd often make sense to talk about the absence of space (one example use-case given below). I'd be happy to include a PR if it was accidentally left out.

My use-case:

if current_user.bandwidth_kb >= 0
  space_remaining = Filesize.from("#{current_user.bandwidth_kb}KB").pretty
else
  space_remaining = "-" + Filesize.from("#{current_user.bandwidth_kb.abs}KB").pretty
end

You have <%= space_remaining %> of bandwidth remaining.

I would assume negative filesizes are a lot harder to convert from one format to another; is that true? I'd of course love to be able to parse negative filesizes and pretty-print them. :+1:

Reproduction steps:

irb(main):018:0* Filesize.from("50KB").pretty
=> "50.00 kB"
irb(main):019:0> Filesize.from("-50KB").pretty
ArgumentError: Unparseable filesize
    from /app/vendor/bundle/ruby/2.2.0/gems/filesize-0.1.1/lib/filesize.rb:135:in `from'
    from (irb):19
    from /app/vendor/bundle/ruby/2.2.0/gems/railties-4.2.7.1/lib/rails/commands/console.rb:110:in `start'
    from /app/vendor/bundle/ruby/2.2.0/gems/railties-4.2.7.1/lib/rails/commands/console.rb:9:in `start'
    from /app/vendor/bundle/ruby/2.2.0/gems/railties-4.2.7.1/lib/rails/commands/commands_tasks.rb:68:in `console'
    from /app/vendor/bundle/ruby/2.2.0/gems/railties-4.2.7.1/lib/rails/commands/commands_tasks.rb:39:in `run_command!'
    from /app/vendor/bundle/ruby/2.2.0/gems/railties-4.2.7.1/lib/rails/commands.rb:17:in `<top (required)>'
    from bin/rails:8:in `require'
    from bin/rails:8:in `<main>'
irb(main):020:0>
dominikh commented 7 years ago

I don't see any reason not to support negative sizes. Via Filesize.new, one can already provide negative sizes; however they won't pretty print correctly, as the formula used assumes positive sizes.

I'd accept a PR that adds parsing of negative sizes, and correctly handles them in pretty printing.

Unrelated: You probably shouldn't be using #from to create these file size objects. You already have a numeric value, current_user.bandwidth_kb and a known unit, KB. It'd make much more sense to use Filesize.new(current_user.bandwidth_kb * 1000) here and avoid the whole parsing machinery.