Switch to martinohmann difflib fork and colorize diff output

[kpengboy]

This would at least be a partial solution to #371.

I did not add the colorization to the cfgtest output, but I wonder if I should.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 15.15152% with 28 lines in your changes missing coverage. Please review.

Project coverage is 37.72%. Comparing base (0739f1f) to head (299fa73).

Files with missing lines	Patch %	Lines
cmd/gmailctl/cmd/apply_cmd.go	0.00%	8 Missing :warning:
cmd/gmailctl/cmd/diff_cmd.go	0.00%	7 Missing :warning:
cmd/gmailctl/cmd/edit_cmd.go	0.00%	7 Missing :warning:
internal/engine/apply/apply.go	0.00%	3 Missing :warning:
internal/engine/label/diff.go	0.00%	3 Missing :warning:

:x: Your patch check has failed because the patch coverage (15.15%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #423      +/-   ##
==========================================
- Coverage   37.81%   37.72%   -0.10%     
==========================================
  Files          53       53              
  Lines        4458     4472      +14     
==========================================
+ Hits         1686     1687       +1     
- Misses       2685     2698      +13     
  Partials       87       87              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions[bot]]

This pull request is stale because it has been open for 30 days without activity. This will be closed in 7 days, unless you add the 'lifecycle/keep-alive' label or comment.

[kpengboy]

keepalive

[kpengboy]

Sorry about that. I fixed it so label diffs respect --color too.

Also, would you want a --color=always option to be implemented? If so we would have to implement the colorization logic ourselves instead of using the martinohmann difflib fork, but it doesn't seem very complicated to do

[mbrt]

@kpengboy I think this looks good enough, thanks! Could you please rebase, resolve the conflicts and fix the tests? After that we can merge

[kpengboy]

Rebased.

[mbrt]

Could you please fix the tests?

[kpengboy]

Fixed the tests. Also, I wrote an alternative version of this PR which supports --color=always by implementing the diff colorization logic in-application (which was not too complex) rather than using the martinohmann library. This is the code for it: https://github.com/mbrt/gmailctl/compare/master...kpengboy:gmailctl:color-diffs-2. If you'd also prefer that, we could use that branch instead of this PR.

[mbrt]

@kpengboy I think it's fine to move to the slightly newer (but still unmaintained) version from martinhomann. I'm also fine if you prefer to move the implementation of the colorization detection to inside here. I checked the branch and it looks pretty simple.

Just one comment on the branch:

I would prefer to keep the reporting package agnostic of flags. This means that the only functions exported would be: ColorizeDiff and IsTerminalColorized that just returns true when the terminal supports coloring.

The logic of parsing the --color flag and deciding what to do can be delegated to a function in cmd/gmailctl/cmd/io.go. This way you can also hardcode the name of the flag in the error message and don't have to pass it around. This keeps a clear separation of concerns, as flags logic never leaves the cmd packages.

I hope this makes sense.

[github-actions[bot]]

This pull request is stale because it has been open for 30 days without activity. This will be closed in 7 days, unless you add the 'lifecycle/keep-alive' label or comment.

[mbrt]

keep alive

[kpengboy]

I've chosen to move to the alternative implementation. I moved the shouldUseColorDiff function from the reporting package to the cmd package too. PTAL

[mbrt]

Nice job! Thanks @kpengboy!