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 run formatting support #48

Merged
merged 6 commits into from
Dec 23, 2019

Conversation

AlexanderVatov
Copy link

I have implemented support for adding runs with formatting (bold, italic, underline, superscript, subscript, small caps, and shadow are supported).

@amiremohamadi
Copy link
Owner

amiremohamadi commented Dec 21, 2019

@AleksanderVatov thanks for the PR! 😃
It seems good to me but there's a few things:

  1. we prefer to name variables and method parameters snake_case
  2. we prefer to only write the type of the parameter in method declaration. so it would be good to avoid writing parameter names in .hpp files
  3. it would be great to create a new .cpp file in samples/ directory and write an example of how to use run-formatting

@AlexanderVatov
Copy link
Author

AlexanderVatov commented Dec 21, 2019

Will do. Can I combine this with the issue I wrote about, with the leading and trailing spaces in runs?

As for the parameter name, I noticed that you don't include them in the header file, but here in particular I thought it might be a good idea because its purpose (as a combination of formatting flags) is otherwise unclear. I can then overload the & and | operators and use the FormattingFlags type.

@AlexanderVatov
Copy link
Author

I overloaded the &, |, &=, and |= operators; now FormattingFlags can be used instead of int, and thus the parameter's name is no longer necessary in the header. I also added a sample.

@AlexanderVatov
Copy link
Author

AlexanderVatov commented Dec 22, 2019

@amiremohamadi I have fixed #50 and implemented two new methods in another branch (https://github.com/AleksanderVatov/DuckX/tree/append_paragraph); please accept this PR first I'll make a new one for that branch.

@amiremohamadi
Copy link
Owner

Can I combine this with the issue I wrote about, with the leading and trailing spaces in runs?

It's better to open separate pull-requests but I'll merge this pull-request soon
Thanks a lot for your efforts :-)

@amiremohamadi amiremohamadi merged commit 0bb53ff into amiremohamadi:master Dec 23, 2019
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.

2 participants