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

fix and maint: match most git supported color settings #453

Merged
merged 4 commits into from Jun 9, 2023

Conversation

webstech
Copy link
Contributor

@webstech webstech commented Jan 28, 2023

A patch series to incrementally expand support for git supported color settings.

  1. maint: set ansi colors in common routine
    Setting foreground and background colors is now using the same code.

  2. fix: ignore negative color attributes with tests
    The colors in the git config may have attributes and negative versions of them (no/no- prefix). The negative versions will no longer cause a false positive checking for the attribute. While not immediately obvious, upcoming support for default(ul) is also affected.

    A new test file is specifically for testing the settings of the ansi codes and the possible settings in the git config.

  3. fix: support git config colors normal and default
    Normal is a placeholder and will be ignored. Default will generate ansi codes for the default foreground or background color.

    Git validates the config so a bright prefix on these values would not be allowed. They are added to the hash to keep the code simple.

  4. fix: support #rgb colors in git config
    Git supports diff colors in #rrggbb notation. diff-so-fancy now supports them as well.

Some of the functions and constants in the head of test file git-config.bats can be moved to util.bash if that is preferred. They are not specific to that file. They are helpers to allow a more generic specification of the ansi escape codes without actually having to know the specific values.

Setting foreground and background colors is now using the same code.

Signed-off-by: Chris. Webster <chris@webstech.net>
The colors in the git config may have attributes and negative versions
of them (no/no- prefix).  The negative versions will no longer cause a
false positive checking for the attribute.

A new test file is specifically for testing the settings of the ansi
codes and the possible settings in the git config.

Signed-off-by: Chris. Webster <chris@webstech.net>
@webstech
Copy link
Contributor Author

webstech commented Jan 28, 2023

This is a prelude to support for default and normal pseudo colors and rgb format colors. Decided to do a separate PR to allow any issues with the test file to be resolved first.

Edit: PR description and contents updated to include this support.

@webstech webstech changed the title fix and maint: fix and maint: ignore negative colors and make code common Jan 30, 2023
@scottchiefbaker
Copy link
Contributor

I'll have to see how all the code pans out. I'm not gonna lie, this is a pretty low-demand feature request. If it's going to complicate the code and potentially hinder maintenance in the future I may pass on it.

My #1 goal with this project (and we're not there yet) is to make the code as readable as possible. The guts of d-s-f are pretty complex, and I'm hesitant to add MORE complexity for an issue (RGB colors) that only a handful of users have requested.

If we can keep the code clean and lower the maintenance cost I'm all for it.

Normal is a placeholder and will be ignored.  Default will generate
ansi codes for the default foreground or background color.

Git validates the config so a bright prefix on these values would not be
allowed.  They are added to the hash to keep the code simple.

Signed-off-by: Chris. Webster <chris@webstech.net>
Git supports diff colors in #rrggbb notation.   diff-so-fancy now
supports them as well.

Signed-off-by: Chris. Webster <chris@webstech.net>
@webstech webstech changed the title fix and maint: ignore negative colors and make code common fix and maint: match most git supported color settings Feb 12, 2023
@webstech
Copy link
Contributor Author

I'll have to see how all the code pans out.

Updated the PR to go all the way with git config compatibility. RGB support was not a big change.

If we can keep the code clean and lower the maintenance cost I'm all for it.

The common routine for set_ansi_color removed duplicate code and made the last two commits simpler.

I know the new test script is a bit different with the tests but the intention is to make it easier to write tests without knowing the internal values for colors. QA test developers should not have to know all the internals.

@scottchiefbaker
Copy link
Contributor

Circling back to this... this looks really good I like it and am gonna merge it.

@scottchiefbaker scottchiefbaker merged commit 8949ad9 into so-fancy:next Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants