Skip to content

Add Ansi 256 Colors support for ResourceBanner #18264

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Add Ansi 256 Colors support for ResourceBanner #18264

wants to merge 4 commits into from

Conversation

@making
Copy link
Member

@making making commented Sep 18, 2019 •
edited
Loading

This commit supports Ansi 256 Colors for ResourceBanner.

${Ansi256Color.Foreground_207}  .   ____          _            __ _ _
${Ansi256Color.Foreground_197} /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
${Ansi256Color.Foreground_220}( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
${Ansi256Color.Foreground_118} \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
${Ansi256Color.Foreground_29}  '  |____| .__|_| |_|_| |_\__, | / / / /
${Ansi256Color.Foreground_57} =========|_|==============|___/=/_/_/_/
${AnsiStyle.NORMAL}

The banner.txt above will print the following banner.

image

Here is background version

${Ansi256Color.Background_207}  .   ____          _            __ _ _
${Ansi256Color.Background_197} /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
${Ansi256Color.Background_220}( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
${Ansi256Color.Background_118} \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
${Ansi256Color.Background_29}  '  |____| .__|_| |_|_| |_\__, | / / / /
${Ansi256Color.Background_57} =========|_|==============|___/=/_/_/_/
${AnsiStyle.NORMAL}

image

This feature is requested by Zipkin(openzipkin/zipkin#2738) which has recently changed the logo color to orange.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 18, 2019
making referenced this pull request in openzipkin/zipkin Sep 18, 2019
@making
Add TODO comment
edebc31 
#2738
@snicoll
Copy link
Member

snicoll commented Sep 18, 2019

@making thanks for the PR. In the future, please build the project locally rather than relying on our CI as every commit triggers a notification to numerous users watching this repository.

@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 19, 2019
@philwebb philwebb added this to the 2.2.x milestone Sep 19, 2019
wilkinsona
wilkinsona reviewed Sep 20, 2019
View reviewed changes
...g-boot-project/spring-boot/src/test/java/org/springframework/boot/ansi/Ansi256ColorTest.java
*
* @author Toshiaki Maki
*/
class Ansi256ColorTest {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be named Ansi256ColorTests

...oject/spring-boot/src/test/java/org/springframework/boot/ansi/Ansi256PropertySourceTest.java
*
* @author Toshiaki Maki
*/
class Ansi256PropertySourceTest {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be named Ansi256PropertySourceTests

...t-project/spring-boot/src/main/java/org/springframework/boot/ansi/Ansi256PropertySource.java
* @author Toshiaki Maki
* @since 2.2.0
*/
public class Ansi256PropertySource extends PropertySource<AnsiElement> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be better to roll this into the existing AnsiPropertySource class?

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Sep 20, 2019
@philwebb philwebb added the for: merge-with-amendments Needs some changes when we merge label Sep 20, 2019
@philwebb philwebb self-assigned this Sep 20, 2019
@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Sep 20, 2019
@philwebb philwebb modified the milestones: 2.2.x, 2.2.0.RC1 Sep 22, 2019
philwebb pushed a commit that referenced this pull request Sep 22, 2019
@making @philwebb
Add ANSI 8-bit color support
65a27ef 
Update ANSI property support to include an 8-bit (256 color) option.

See gh-18264
philwebb added a commit that referenced this pull request Sep 22, 2019
@philwebb
Polish 'Add ANSI 8-bit color support'
7f79c26 
See gh-18264
philwebb added a commit that referenced this pull request Sep 22, 2019
@philwebb
Add ANSI 8-bit color image banner support
4ef1e18 
Update `ImageBanner` and `AnsiColors` to optionally support 8-bit
color output.

See gh-18264
@philwebb philwebb closed this in 18396a3 Sep 22, 2019
@philwebb
Copy link
Member

Thanks a lot @making, this has now been merged to master. I've made a few changes in commit 7f79c26 to save the need for an additional property source. You now use the form ${AnsiColor.207} or ${AnsiBackground.207} if you want to pick an 8-bit color. I've also update the ImageBanner code to support more colors.

@philwebb philwebb mentioned this pull request Sep 22, 2019
Closed
pull bot pushed a commit to scope-demo/spring-boot that referenced this pull request Sep 22, 2019
@making @pull
Add ANSI 8-bit color support
802fba0 
Update ANSI property support to include an 8-bit (256 color) option.

See spring-projectsgh-18264
pull bot pushed a commit to scope-demo/spring-boot that referenced this pull request Sep 22, 2019
@philwebb @pull
Polish 'Add ANSI 8-bit color support'
71c6a24 
See spring-projectsgh-18264
pull bot pushed a commit to scope-demo/spring-boot that referenced this pull request Sep 22, 2019
@philwebb @pull
Add ANSI 8-bit color image banner support
2deac0e 
Update `ImageBanner` and `AnsiColors` to optionally support 8-bit
color output.

See spring-projectsgh-18264
@making
Copy link
Member Author

making commented Sep 22, 2019

@philwebb Thanks for the cool refactoring 😍!

@making making deleted the add-ansi256-color branch September 22, 2019 07:39
@making making mentioned this pull request Sep 22, 2019
Merged
@philwebb
Copy link
Member

I've also added block support in #18301 which helps make nicer images like thisL

Screen Shot 2019-09-22 at 1 54 07 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wilkinsona wilkinsona wilkinsona left review comments

Assignees

@philwebb philwebb

Labels

for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement

Projects

None yet

Milestone

2.2.0.RC1

Development

Successfully merging this pull request may close these issues.

5 participants

@making @snicoll @philwebb @wilkinsona @spring-projects-issues