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

Make up lines even more beautiful #11770

Merged
merged 1 commit into from Oct 8, 2018

Conversation

jdz
Copy link
Contributor

@jdz jdz commented Oct 8, 2018

Use dashed line from the same Unicode block as other line drawing
characters. This ensures that they all are present in the same font,
are the same width and align perfectly.

Re-opening #11727 according to guidelines.

Use dashed line from the same Unicode block as other line drawing
characters.  This ensures that they all are present in the same font,
are the same width and align perfectly.
@jdz
Copy link
Contributor Author

jdz commented Oct 8, 2018

3/4 tests failed because r2r PR was (initially) not rebased on top of master.

@codecov-io
Copy link

Codecov Report

Merging #11770 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11770      +/-   ##
==========================================
+ Coverage   37.02%   37.03%   +<.01%     
==========================================
  Files         901      901              
  Lines      288124   288082      -42     
==========================================
- Hits       106692   106688       -4     
+ Misses     181432   181394      -38
Impacted Files Coverage Δ
libr/core/disasm.c 73.46% <ø> (ø) ⬆️
libr/asm/p/asm_m68k_cs.c 71.01% <0%> (-0.82%) ⬇️
binr/radare2/radare2.c 49.88% <0%> (-0.06%) ⬇️
libr/core/linux_heap_glibc.c 15.9% <0%> (-0.04%) ⬇️
libr/asm/arch/v850/v850_disas.h 0% <0%> (ø) ⬆️
libr/asm/p/asm_lanai_gnu.c 0% <0%> (ø) ⬆️
libr/magic/magic.c 34.3% <0%> (ø) ⬆️
libr/core/panels.c 0% <0%> (ø) ⬆️
libr/anal/p/anal_v850.c 0% <0%> (ø) ⬆️
libr/core/cmd_debug.c 20.66% <0%> (+0.24%) ⬆️
... and 1 more

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 6a75230...26db2c0. Read the comment docs.

@radare radare merged commit 0cd5c5d into radareorg:master Oct 8, 2018
@radare
Copy link
Collaborator

radare commented Oct 8, 2018

uhm, doesnt looks good here
screenshot 2018-10-09 at 00 21 35

@radare
Copy link
Collaborator

radare commented Oct 8, 2018

without curvy
screenshot 2018-10-09 at 00 22 26

@radare
Copy link
Collaborator

radare commented Oct 8, 2018

before was not looking great either..

screenshot 2018-10-09 at 00 23 47

@jdz
Copy link
Contributor Author

jdz commented Oct 9, 2018

What font is this?

It looks like it does not have the "box drawing" Unicode block (the lines are way lighter than the font itself). It seems to me that the missing corner and vertical line characters are pulled from one font, and this font does not have dashed vertical line, so it is pulled from yet another font.

In #11727 I included screenshots with 4 different monospace fonts that do have characters for the whole "box drawing" Unicode block. You might have noticed that I've removed some of the screenshots from the original comment. That's because I later found out that in the other screenshots the box drawing characters were taken from DejaVu Sans Mono.

To summarise: not all fonts have the box drawing characters in the font, but when they do, there's a great chance the whole Unicode block is included. If your favourite font does not have this unicode block then the font substitution must be configured. There's ample information on how to do it on Linux with fontconfig; not sure about MacOS.

I still stand by this change and think it's the right thing to do.

@radare
Copy link
Collaborator

radare commented Oct 12, 2018 via email

@jdz
Copy link
Contributor Author

jdz commented Oct 12, 2018

I use Input: http://input.fontbureau.com/.

@XVilka
Copy link
Contributor

XVilka commented Oct 13, 2018 via email

@radare
Copy link
Collaborator

radare commented Oct 13, 2018 via email

@jdz jdz deleted the improve-unicode-line-graphics branch November 8, 2018 14:33
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

4 participants