Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for colorized values #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbacovsky
Copy link

When I wanted to colorize some values, I used the following formatter

  class ColorFormatter
      def initialize(color)
        @color = color
      end
      def format(data)
        HighLine.color(data.to_s, @color)
      end
    end

The cells were then padded incorrectly, because the unprintable escape sequence that sets the color was counted in the length of the value. this patch is stripping the unprintable stuff during padding.

@arches
Copy link
Owner

arches commented Oct 28, 2013

Colors have been something I've wanted to implement for a long time. I have a couple reservations about your approach. First, the truncation formatter is called so many times that anything slowing it down has a huge performance impact. I'm actually thinking I need to roll back one of the multibyte fixes because it makes the character counting so slow. Second, I worry that keeping a full list of all non-printing characters is going to be difficult.

Right now there's one set of user-supplied formatters, which are applied in sequence, followed by three table_print-supplied formatters (time, no-newlines, and fixed-width). How would you feel about a second set of user-supplied formatters which are run AFTER those three? I'm assuming they'd mostly be for coloring and other non-printing manipulation, since changing the strings after they're truncated would misalign the table.

If it helps I'm also thinking of passing the original value to the formatters along with the current value. That way the colorizer wouldn't have to try to use the truncated value to figure out what color to use.

Would that solution address your use cases?

@arches
Copy link
Owner

arches commented Oct 28, 2013

btw, thanks for the patch and for reminding me to look at it. colors are something that's been a nice-to-have for me for MONTHS but I've never been able to find the time to tackle it, so it's great to have a little push from someone else :)

@mbacovsky
Copy link
Author

Adding second level of formatters would solve my problem with coloring and it would help if the formatter can have both values (the orignal one and the current one).

I no longer need this feature in my project but still it would be nice to have. If it makes sense to rebase this PR please let me know. Feel free to close it otherwise.

Thanks for all your work on table_print.

@shlomizadok
Copy link

shlomizadok commented Sep 29, 2016

Hey,
I would love to have this merged in, instead of (ugly) monkey patching @ theforeman/hammer-cli#218

One comment though:

def strip_escape(value)
  value.gsub(/[\u0080-\u00ff]/, ' ')
end

worked better for me

@arches
Copy link
Owner

arches commented Sep 29, 2016

I've been working on a refactor to separate the formatting and the structural components, which will give us a lot more flexibility on the formatting and make changes easier in general. The new version has explicit formatter classes and prints off of config instances rather than having a single global config. It also opens the door for full support of other formats (csv, html).

I was planning to do everything in one phase, but I'll rearrange my focus and break it out a bit. Phase 1: get the refactor to a place that's stable (if incomplete) and addresses the colorizing use case.

@shlomizadok
Copy link

@arches, thanks for your answer and for this awesome gem.
Please also note that this issue also solves #2, so it may be worth adding it on the next iteration
(and I thing it would be easier to break it to smaller phrases...)

ccorn90 added a commit to ccorn90/table_print that referenced this pull request Jan 14, 2018
@drush
Copy link

drush commented Jan 28, 2022

Display width can be calculated on chars and strings accurately. I haven't spent a lot of time on this, but I'll leave this here for those that might be looking at the issue https://stackoverflow.com/questions/10021591/counting-unicode-string-length-without-combining-marks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants