color icon indicating copy to clipboard operation
color copied to clipboard

tag-style color rendering fails if lines contain tag like content

Open TLINDEN opened this issue 3 years ago • 5 comments

System (please complete the following information):

  • OS: linux
  • GO Version: 1.18.1
  • Pkg Version: 1.5.2

ENV info on the terminal (by command env | grep -i TERM):

COLORTERM=truecolor
TERM_PROGRAM_VERSION=3.2a
TERM=tmux-256color
TERM_PROGRAM=tmux

Describe the bug

I am using tag-style color rendeing to colorize tabular data. If multiple lines contain something like this: <none> then only the first colorziation works, subsequent ones are not replaced but printed as is.

To Reproduce

package main

import (
	"github.com/gookit/color"
)

func main() {
	test1 := `FAILS:
one <bg=lightGreen;fg=black>two</> <three>
foo <bg=lightGreen;fg=black>two</> <four>

`

	test2 := `WORKS:
one <bg=lightGreen;fg=black>two <three></>
foo <bg=lightGreen;fg=black>two <four></>

`

	test3 := `WORKS:
one <bg=lightGreen;fg=black>two</> three
foo <bg=lightGreen;fg=black>two</> four
`

	color.Print(test1)
	color.Print(test2)
	color.Print(test3)
}

Expected behavior

The colorziation should work independently from other non color-tag content.

Screenshots

See screenshot attached.

colorfails

** Edited to add **

The problem occurs ONLY if the "tagged" content appears outside the colorization. If it's inside, it works. Sample code and screenshot updated.

TLINDEN avatar Oct 24 '22 10:10 TLINDEN

https://github.com/gookit/color/pull/53 fixes the issue for me.

TLINDEN avatar Oct 24 '22 11:10 TLINDEN

hi @TLINDEN

FAILS:
one <bg=lightGreen;fg=black>two</> <three>
foo <bg=lightGreen;fg=black>two</> <four>

On fails, will match: <bg=lightGreen;fg=black>two</> <three>\nfoo <bg=lightGreen;fg=black>two</> ... Maybe is right, because color support match like <info> contents </>

If change regex s to m, will be not match mulit line contents:

foo <bg=lightGreen;fg=black>
two
</>

inhere avatar Oct 24 '22 14:10 inhere

But then, there's still an issue, since <three> is no valid color tag. However - even if it were, the color module shall throw an error because there's no matching </> closing tag for it. And it's also too greedy, from my understanding as a user, it should be:

  1. match an opening tag, either <$name> or <bg|fg...>
  2. match the smallest possible part til the next part
  3. match the closing tag </>

Or, if we translate the example into HTML:

one <b>two</b> &lt;three&gt;
foo <b>two</b> &lt;four&gt;

You'll get: one two <three> foo two <four>

But not: one two <three> foo two <four>

Now, I see that the s flag needs to be there in order to be able to match color tags spanning multiple lines. And since GO doesn't support negative lookahead lookups (which would fix it!), how can both cases be satisfied?

TLINDEN avatar Oct 24 '22 15:10 TLINDEN

Ok, while I still think that this behavior is somewhat buggy, I resolved my issue by using regexp.ReplaceAllStringFunc() and inside I am using the color.Style.Sprint() function directly. That way I don't have to cope with any disambiguities.

So, from my point of view the case could be closed. Thanks for the fast response!

TLINDEN avatar Oct 25 '22 12:10 TLINDEN

Thanks you I didn't think of a good way :)

Maybe ... can direct limit all color tag name. eg: info|red|yellow....

But then, there's still an issue, since <three> is no valid color tag. However - even if it were, the color module shall throw an error because there's no matching </> closing tag for it. And it's also too greedy, from my understanding as a user, it should be:

  1. match an opening tag, either <$name> or <bg|fg...>
  2. match the smallest possible part til the next part
  3. match the closing tag </>

Or, if we translate the example into HTML:

one <b>two</b> &lt;three&gt;
foo <b>two</b> &lt;four&gt;

You'll get: one two foo two

But not: one two foo two

Now, I see that the s flag needs to be there in order to be able to match color tags spanning multiple lines. And since GO doesn't support negative lookahead lookups (which would fix it!), how can both cases be satisfied?

inhere avatar Oct 25 '22 12:10 inhere