bisync: coloring in the logs not obeying --color or redirection

[ncw]

The coloring of the bisync logs isn't working the same as the coloring in the rest of rclone.

The coloring is supposed to be removed when you write to a file or it isn't supported. On Windows the coloring is done a completely different way (not with escape codes, except on Windows 10 or greater I think).

That is why we see this sort of thing in the log

2024/04/16 01:17:44 �[36m(01)  :�[0m �[34mtest extended-char-paths�[0m

If we were using terminal.Out to write to the terminal then this would be working properly and the coloring would also be obeying

 --color AUTO|NEVER|ALWAYS   When to show colors (and other ANSI codes) AUTO|NEVER|ALWAYS (default AUTO)

We are however using colors in rclone's logging system which was never intended to work like that! We probably want to continue using the log system for bisync so we need an official way of introducing colors into the log.

To fix that we could potentially make a new set of terminal colors, so log.RedFg() say (in fs/log) which is a function which will return empty string instead of the escape sequence as appropriate.

What do you think @nielash ?

How to use GitHub

• Please use the 👍 reaction to show that you are affected by the same issue.
• Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
• Subscribe to receive notifications on status change and new comments.

[nielash]

bisync should be respecting --color. Are you able to reproduce it outside of the tests? On the tests we deliberately hard-code ALWAYS so that the colors themselves are tested (any unexpected color differences are reported as failures):

rclone/cmd/bisync/bisync_test.go

Line 272 in 8dc4c01

bisync.Colors = true

We are however using colors in rclone's logging system which was never intended to work like that! We probably want to continue using the log system for bisync so we need an official way of introducing colors into the log.

To fix that we could potentially make a new set of terminal colors, so log.RedFg() say (in fs/log) which is a function which will return empty string instead of the escape sequence as appropriate.

I stumbled over that a few months ago and added this as a quick fix: 8d3bcc0

But I agree it would be better to handle this more properly in fs/log. I think your idea sounds good (if I'm understanding it correctly!)