Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add simple --diff colour support #14491

Merged
merged 2 commits into from
Feb 18, 2016
Merged

Add simple --diff colour support #14491

merged 2 commits into from
Feb 18, 2016

Conversation

gechr
Copy link
Contributor

@gechr gechr commented Feb 15, 2016

Happy to extend this to make diff output more smart/configurable in future, but wanted to get the ball rolling with hardcoded colours as this (in my opinion!) makes --diff output considerably easier to visually parse.

Since it uses stringc from ansible.utils.color, it naturally obeys the standard ANSIBLE_[NO|FORCE]COLOR constants.

I note there's currently another PR open (#14434) which modifies the surrounding code, so if you'd like me to rebase this on top of that to make things easier, let me know.


Screenshot

diff colours

@bcoca bcoca added this to the 2.1.0 milestone Feb 15, 2016
@bcoca
Copy link
Member

bcoca commented Feb 15, 2016

oohhh ... pretty ....

@mgedmin
Copy link
Contributor

mgedmin commented Feb 17, 2016

❤️

@mgedmin
Copy link
Contributor

mgedmin commented Feb 17, 2016

BTW have you checked whether it looks readable on terminals that use white background?

@gechr
Copy link
Contributor Author

gechr commented Feb 17, 2016

@mgedmin: I haven't, because, though configurable, the colours I chose are already in use by default (green=ok, red=failed, cyan=skipped). Colours can also be tweaked in the user's terminal preferences if they're using a non-black background.

With that said, I'll push a change to make the colours configurable in Ansible as well - variety is the spice of life!

@mgedmin
Copy link
Contributor

mgedmin commented Feb 17, 2016

I haven't, because, though configurable, the colours I chose are already in use by default (green=changed, red=failed, cyan=skipped).

Ah. In that case I can confirm that the colours are perfectly readable.

bcoca added a commit that referenced this pull request Feb 18, 2016
@bcoca bcoca merged commit 5300a2e into ansible:devel Feb 18, 2016
@gechr gechr deleted the gc-simple-colour-diff branch February 18, 2016 10:10
@towolf
Copy link
Contributor

towolf commented Feb 18, 2016

Thanks a lot for checking off an item from my personal whiteboard:

The second one today actually.

@bcoca, now that --diff is a "Major Change" on the CHANGELOG for 2.1.0, can you please get my PRs merged that add diff mode to apt: and lineinfile:?

ansible/ansible-modules-core/pull/2944
ansible/ansible-modules-core/pull/2896

@mgedmin
Copy link
Contributor

mgedmin commented Feb 18, 2016

@bcoca also my "No newline at EOF" --diff fix from #14434 ;)

@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants