Feat: Added --disableANSI Flag to CLI to enable/disable ANSI Color Codes

Akash-Singh04 commented 3 months ago

Related Issue

Added --enableANSI CLI Flag to enable/disable colour encoding.

Closes: #1520

Describe the changes you've made

Added --enableANSIColor PersistantFlag to CLI to enable/disable ASNI Color Codes Example of enabled ANSI Colors in Log files:
Added it to testCmd inorder to prevent ASNI Color Codes being rendered in this part of report:

Type of change

[x] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Code style update (formatting, local variables)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Please let us know if any test cases are added

NIL

Describe if there is any unusual behaviour of your code(Write `NA` if there isn't)

NIL

Checklist:

[x] My code follows the style guidelines of this project.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[ ] I have made corresponding changes to the documentation.
[x] My changes generate no new warnings.
[x] I have added tests that prove my fix is effective or that my feature works.
[ ] New and existing unit tests pass locally with my changes.

Screenshots (if any)

Usage:

Original	Updated
**

| | |

** |

|

Below is an attached video of the implementation in action:

https://github.com/keploy/keploy/assets/114267538/eac44ce0-fbfb-41a1-90a8-b62d908e22a5

Akash-Singh04 commented 3 months ago

@slayerjain Kindly review the PR and let me know if any changes are required.

PranshuSrivastava commented 3 months ago

Also fix the merge conflicts please

Akash-Singh04 commented 3 months ago

@PranshuSrivastava Kindly review this PR and have a look at the attached video to better understand the implementation, Thank You!

PranshuSrivastava commented 3 months ago

Also please fix the merge conflicts

charankamarapu commented 2 months ago

Hey @Akash-Singh04 please update your branch with the refactoring changes. Mostly the code where the color is decided is same. Major change would be around cli. you can add the flag in this file https://github.com/keploy/keploy/blob/7febf76bd7b1230604d9b8afb5add32e703ba34d/cli/provider/cmd.go#L153 please add the flag in the config also and I think it is better to use persistent flag for this flag. You can discuss with the reviewer and go with.

Akash-Singh04 commented 2 months ago

@charankamarapu @PranshuSrivastava The pr is ready after refactoring! Do let me know if there are any further deliverables

PranshuSrivastava commented 2 months ago

Looks the color is not being changed in the correct place, I am still getting colored logs in the beginning

Akash-Singh04 commented 2 months ago

Looks the color is not being changed in the correct place, I am still getting colored logs in the beginning

The reason being that the logger is initially set-up here: func NewColor(cfg zapcore.EncoderConfig, enableColor bool) (enc zapcore.Encoder) { if enableColor { return color{ EncoderConfig: &cfg, Encoder: zapcore.NewConsoleEncoder(cfg), } } // fmt.Println("Color is disabled") return zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()) }

Then when the command runs, and the flags are read, we change the color of the logger: if !c.cfg.EnableANSIColor { c.logger.Info("Color encoding is disabled") logger, err := log.ChangeColorEncoding() *c.logger = *logger if err != nil { errMsg := "failed to change color encoding" utils.LogError(c.logger, err, errMsg) return errors.New(errMsg) } }

This is due to the current flow of commands in the v2 codebase. For the most part it correctly disables the ANSI color codes except in the beginning while the flags are being setup.

CC: @PranshuSrivastava

PranshuSrivastava commented 1 month ago

Ok, the first log can be left colored as we haven't fully read the flags by that time, but the second log is being printed by you right? So why can't you just print it after changing the colors instead of before it?

Akash-Singh04 commented 1 month ago

@PranshuSrivastava Your requested changes have been made

Akash-Singh04 commented 1 month ago

@PranshuSrivastava Your requested changes have been made

re-Tick commented 1 month ago

Hey @Akash-Singh04, I think the flag should be disableANSIColor since, the value for the enableANSIColor is true by default. Instead the value can be false and flag can be --disableANSIColor. @PranshuSrivastava would that be ok right?

PranshuSrivastava commented 1 month ago

@re-Tick Yes thats good. I just want the name to be short

Akash-Singh04 commented 1 month ago

@re-Tick @PranshuSrivastava Your requested changes have been made

keploy / keploy