[WIP]: Apply basic colorization to diff

Review Request #11829 — Created Oct. 3, 2021 and updated

sandysaji
RBTools
master
rbtools

Add an option to the rbt diff command to
apply basic colorization to the output.
The rbt diff --color command highlights
removed lines in red and added lines in green.

This review request includes the following:

  • Utilizing pygments library, format the diff displayed
    on terminal with basic colorization.
  • Add --color option to rbt diff command.

Work in progress.

Currently, there are not test cases for rbt diff command.
Therefore, we will need to write test cases for the command.

Summary
Apply basic colorization to diff
clean up
Update code to meet PEP8 coding standards
Update code to meet PEP8 coding standards
Update code to meet PEP8 coding standards
Updated if-else statement with elif
clean up file
Inital test for rbt diff command
fix flake8 error and minor update to test file
Update color option to utilize choices instead of boolean
remove print statements
fix flake8 errors
fix flake8 errors

Description From Last Updated

E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E501 line too long (94 > 79 characters)

reviewbotreviewbot

E124 closing bracket does not match visual indentation

reviewbotreviewbot

E501 line too long (94 > 79 characters)

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

It might be nicer to do elif six.PY2 and then an else instead of having nested if else statements

maubinmaubin

F841 local variable 'diff' is assigned to but never used

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

W292 no newline at end of file

reviewbotreviewbot

Instead of just having this be a boolean, I think we should follow git's lead and have it be a ...

daviddavid

F841 local variable 'diff' is assigned to but never used

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

F841 local variable 'diff' is assigned to but never used

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

F841 local variable 'diff' is assigned to but never used

reviewbotreviewbot

F841 local variable 'diff' is assigned to but never used

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

sandysaji
Review request changed

Commits:

Summary
-
Apply basic colorization to diff
-
clean up
+
Apply basic colorization to diff
+
clean up
+
Update code to meet PEP8 coding standards

Diff:

Revision 2 (+66 -20)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

sandysaji
Review request changed

Commits:

Summary
-
Apply basic colorization to diff
-
clean up
-
Update code to meet PEP8 coding standards
+
Apply basic colorization to diff
+
clean up
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards

Diff:

Revision 3 (+74 -22)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

sandysaji
sandysaji
maubin
  1. Overall looks good :)

  2. rbtools/commands/diff.py (Diff revision 4)
     
     

    It might be nicer to do elif six.PY2 and then an else instead of having nested if else statements

  3. 
      
sandysaji
Review request changed

Commits:

Summary
-
Apply basic colorization to diff
-
clean up
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
+
Apply basic colorization to diff
+
clean up
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Updated if-else statement with elif
+
clean up file
+
Inital test for rbt diff command

Diff:

Revision 5 (+169 -29)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

sandysaji
Review request changed

Commits:

Summary
-
Apply basic colorization to diff
-
clean up
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Updated if-else statement with elif
-
clean up file
-
Inital test for rbt diff command
+
Apply basic colorization to diff
+
clean up
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Updated if-else statement with elif
+
clean up file
+
Inital test for rbt diff command
+
fix flake8 error and minor update to test file

Diff:

Revision 6 (+169 -33)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. 
      
  2. rbtools/commands/diff.py (Diff revision 6)
     
     
     

    Instead of just having this be a boolean, I think we should follow git's lead and have it be a choice between "never", "always", and "auto" (with auto being the default). When using auto, we should check sys.stdout.isatty() to determine whether to use color.

  3. 
      
sandysaji
Review request changed

Commits:

Summary
-
Apply basic colorization to diff
-
clean up
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Updated if-else statement with elif
-
clean up file
-
Inital test for rbt diff command
-
fix flake8 error and minor update to test file
+
Apply basic colorization to diff
+
clean up
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Updated if-else statement with elif
+
clean up file
+
Inital test for rbt diff command
+
fix flake8 error and minor update to test file
+
Update color option to utilize choices instead of boolean
+
remove print statements

Diff:

Revision 7 (+209 -49)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

sandysaji
Review request changed

Commits:

Summary
-
Apply basic colorization to diff
-
clean up
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Updated if-else statement with elif
-
clean up file
-
Inital test for rbt diff command
-
fix flake8 error and minor update to test file
-
Update color option to utilize choices instead of boolean
-
remove print statements
+
Apply basic colorization to diff
+
clean up
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Updated if-else statement with elif
+
clean up file
+
Inital test for rbt diff command
+
fix flake8 error and minor update to test file
+
Update color option to utilize choices instead of boolean
+
remove print statements
+
fix flake8 errors

Diff:

Revision 8 (+213 -51)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

sandysaji
Review request changed

Commits:

Summary
-
Apply basic colorization to diff
-
clean up
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Update code to meet PEP8 coding standards
-
Updated if-else statement with elif
-
clean up file
-
Inital test for rbt diff command
-
fix flake8 error and minor update to test file
-
Update color option to utilize choices instead of boolean
-
remove print statements
-
fix flake8 errors
+
Apply basic colorization to diff
+
clean up
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Update code to meet PEP8 coding standards
+
Updated if-else statement with elif
+
clean up file
+
Inital test for rbt diff command
+
fix flake8 error and minor update to test file
+
Update color option to utilize choices instead of boolean
+
remove print statements
+
fix flake8 errors
+
fix flake8 errors

Diff:

Revision 9 (+214 -52)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

Loading...