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

Restyling graph backedges + bring back 't' and 'f' hints #10837

Merged
merged 3 commits into from Jul 29, 2018

Conversation

@cyanpencil
Copy link
Contributor

@cyanpencil cyanpencil commented Jul 27, 2018

  1. Bring back t and f hints in graph. Governed by variable graph.hints. Defaults to True.
    Added command # to toggle hints while in graph view.

  2. Use a different style for backedges.
    I tried using braille characters, but the result was not good (braille points are a 2x4 table, unfortunately all even numbers, lines were decentered and the visual effect was worse than normal hyphens (-)).
    So I ended up using dashed lines for backedges. I think it is the most clear way to differentiate them from both normal edges and conditional edges. Take a look in this screens:

image

On a different terminal with different font, a conditional backedge:

image

@cyanpencil
Copy link
Contributor Author

@cyanpencil cyanpencil commented Jul 27, 2018

For reference, this is how it would have come out with Braille chars:

image

I don't know, there are small problems in the corners and near the arrows, becuase the points are not centered in the terminal cells, for this reason I chose to go with dashes.

Tell me if you think braille looks better to you

Refactoring

Small fix

Fix for windows in graph.c
@cyanpencil cyanpencil force-pushed the cyanpencil:dotted-lines-2 branch from 0566382 to f491e4b Jul 27, 2018
@cyanpencil cyanpencil force-pushed the cyanpencil:dotted-lines-2 branch from d4c3d03 to bdd0ff6 Jul 28, 2018
@Maijin
Copy link
Contributor

@Maijin Maijin commented Jul 28, 2018

Not really fan of t and f by default, i find it looks way cooler with the boxes connected to the lines.
Probably need a vote or something to decide what default should looks like @XVilka ?

@XVilka XVilka merged commit f3b3364 into radareorg:master Jul 29, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants