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

New fonts #188

Merged
merged 17 commits into from
Dec 9, 2021
Merged

New fonts #188

merged 17 commits into from
Dec 9, 2021

Conversation

sepandhaghighi
Copy link
Owner

@sepandhaghighi sepandhaghighi commented Dec 7, 2021 •
edited
Loading

Reference Issues/PRs

#155

What does this implement/fix? Explain your changes.

  • 10 new font added
    1. fancy131
    2. fancy132
    3. fancy133
    4. fancy134
    5. fancy135
    6. fancy136
    7. fancy137
    8. tarty1
    9. tarty2
    10. tarty3
  • mix_letters function modified
  • CONTRIBUTING.md modified

Any other comments?

@sepandhaghighi sepandhaghighi added this to the art 5.4 milestone Dec 7, 2021
@sepandhaghighi sepandhaghighi self-assigned this Dec 7, 2021
@sepandhaghighi
Copy link
Owner Author

@sadrasabouri Please review this PR 💯

sadrasabouri
sadrasabouri reviewed Dec 8, 2021
View reviewed changes
Copy link
Collaborator

@sadrasabouri sadrasabouri left a comment

Choose a reason for hiding this comment

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

Just a tiny comment.

art/text_dic3.py
'V': '𝖁',
'z': '𝖟'}

tarty1_dic = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it better if we sort these lines respected to the alphabetic order (like non-ASCII fonts)?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Which lines? Dictionary's keys?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's not easy (needs a lot of changes!!) and also Python standard dictionary doesn't support order (Python < 3.8).

sadrasabouri
sadrasabouri requested changes Dec 8, 2021
View reviewed changes
Copy link
Collaborator

@sadrasabouri sadrasabouri left a comment •
edited
Loading

Choose a reason for hiding this comment

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

tarty2 seems a little bit wrong:

Screenshot from 2021-12-08 15-57-20

@sepandhaghighi
Copy link
Owner Author

tarty2 seems a little bit wrong:

Screenshot from 2021-12-08 15-57-20

It's a bit strange, let me check it.

@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2021 •
edited
Loading

Codecov Report

Merging #188 (9814dde) into dev (54f686a) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #188      +/-   ##
==========================================
+ Coverage   90.95%   90.98%   +0.04%     
==========================================
  Files           1        1              
  Lines         276      277       +1     
  Branches       75       75              
==========================================
+ Hits          251      252       +1     
  Misses         23       23              
  Partials        2        2
Impacted Files Coverage Δ
art/art.py 90.98% <100.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54f686a...9814dde. Read the comment docs.

@sepandhaghighi
Copy link
Owner Author

@sadrasabouri Please take a look at 5ce091b and 9814dde

@sadrasabouri sadrasabouri merged commit f9396db into dev Dec 9, 2021
@sadrasabouri sadrasabouri deleted the new_fonts branch December 9, 2021 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sadrasabouri sadrasabouri sadrasabouri approved these changes

Assignees

@sepandhaghighi sepandhaghighi

Labels
new font
Projects
None yet
Milestone
art 5.4
Development

Successfully merging this pull request may close these issues.
3 participants
@sepandhaghighi @codecov-commenter @sadrasabouri