tty-table icon indicating copy to clipboard operation
tty-table copied to clipboard

Multiline render option breaks multi coloured cell data

Open markpitchless opened this issue 5 years ago • 4 comments

Spotted an odd bug trying to render multicoloured strings in a cell, tty-tables looks to be getting confused by the end and start of colour codes generated by pastel. Can see it most clearly in :unicode render but it also fails with :basic render.

require "tty-table"

p = Pastel.new
hello = p.decorate("hello",:black, :on_green)
world = p.decorate("world",:black, :on_red)

["#{hello}", "#{hello}#{world}", "#{hello} #{world}"].each do |cell|
  puts cell
  puts cell.inspect
  puts
  tab = TTY::Table.new [:head], [[cell]]
  puts tab.render :unicode, multiline: false
  puts
  puts tab.render :unicode, multiline: true
  puts
end
screen shot 2018-10-22 at 22 13 03

The middle one shows the problem. multiline: true seems to cause tty-table to split hello\e[0m\e[30;41mworld with a newline after [30; but (the 3rd image) it is ok with hello\e[0m \e[30;41mworld (space between coloured bits).

markpitchless avatar Oct 22 '18 20:10 markpitchless

Digging into this I realised it was the wrapping code that got tripped up, it doesn't know how to wrap such a string. Here: https://github.com/piotrmurach/tty-table/blob/master/lib/tty/table/operation/wrapped.rb#L37 The newline appears in the middle :) Took some digging to get there!

Working around with monkey patch in my app:

    module Operation
      class Wrapped
        def call(field, row, col)
          column_width = widths[col] || field.width
          @pastel ||= Pastel.new
          if @pastel.colored? field.content
            field.content
          else
            Strings.wrap(field.content, column_width)
          end
        end
      end
    end

Which just ignores wrapping for colourful strings.

Will work up a proper patch for you, using this https://github.com/alyssais/table/blob/master/lib/ansi/string_view.rb which heroically solves the wrapping coloured text problem I avoided. Hello @alyssais 👋

markpitchless avatar Oct 24 '18 17:10 markpitchless

Hey!

Fwiw, it wouldn’t be possible to include that class of mine in tty-prompt as it stands, because of its licensing (it’s GPLv3). If you want to use it, I’d have to think about whether I want to relicense.

(If you just want to take inspiration from it and basically use it as documentation on ANSI escape handling, but write a new thing that just solves the problem here, that’s a moot point.)

alyssais avatar Oct 24 '18 18:10 alyssais

@markpitchless Thank you for pointing out the issue! This is very helpful. As far as I'm concerned I would be keen for the bug to be addressed directly in the strings dependency and in particular the wrap call. If you have time to code up a patch I would be most appreciative due to my limited time.

@alyssais Hello! As the author/maintainer of tty related gems I can assure you that I don't have the intention of using your gem in any of the tty gems and will not be reading or copying the source from your table gem, I'm already very happy with the strings gem that I've created quite few years back to handle text transformations that include ANSI strings. This is the gem that is at fault in this case and I would rather fix it than seek alternative solutions. If @markpitchless wants to use your solution in his own project to tacle the problem of handling ANSI codes then that's cool but I won't be using it in tty-table. I hope this clarifies things and solves any contentions that may arise.

piotrmurach avatar Oct 25 '18 21:10 piotrmurach

Oh neat, I hadn’t seen the strings gem. That’s awesome!

alyssais avatar Oct 26 '18 07:10 alyssais