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.

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

Add faint, blink, reverse and conceal to the emphases #2394

Merged
merged 1 commit into from
Jul 3, 2021
Merged

Add faint, blink, reverse and conceal to the emphases #2394

merged 1 commit into from
Jul 3, 2021

Conversation

benit8
Copy link
Contributor

@benit8 benit8 commented Jun 27, 2021

Picking up the work of @data-man from #2303, since he had to close his PR because of personal matters.

Closes #2302.

@data-man
Copy link

Thank you, I'm back in line! :)
I think the color output code can be reworked.
And I'm not sure that an internal buffer is needed at all.

@benit8
Copy link
Contributor Author

benit8 commented Jun 29, 2021

Glad to hear you're okay 🙂
Do you want to reopen your PR then ? Or work on another if you think your ideas are too out of scope for this one ?
I was only trying to get faint text for one of my own projects, I don't know if I'll have the time for a big rework 😅

@data-man
Copy link

data-man commented Jul 1, 2021

Do you want to reopen your PR then ?

No.

Or work on another if you think your ideas are too out of scope for this one ?

Yes, but later :)
I'm currently using the Contour terminal and would like to add all of its coloring features to fmt.
But I'm not sure that such PR would be accepted. :)

vitaut
vitaut requested changes Jul 2, 2021
View reviewed changes
include/fmt/color.h Outdated Show resolved Hide resolved
include/fmt/color.h Outdated Show resolved Hide resolved
vitaut
vitaut reviewed Jul 2, 2021
View reviewed changes
include/fmt/color.h Outdated
};

constexpr size_t num_emphases = 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's an internal symbol only used in rgb so let's move it there and make private (and static).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't understand how it is only used in the rgb struct; shouldn't it be in the ansi_color_escape struct only ?

@vitaut vitaut merged commit 02ad5e1 into fmtlib:master Jul 3, 2021
@vitaut
Copy link
Contributor

vitaut commented Jul 3, 2021

Thanks @benit8 and @data-man!

PoetaKodu pushed a commit to pacc-repo/fmt that referenced this pull request Nov 11, 2021
@benit8 @PoetaKodu
Add faint, blink, reverse and conceal to the emphases (fmtlib#2394)
64ae853
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vitaut vitaut vitaut requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add more emphases
3 participants
@benit8 @data-man @vitaut