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

Dfferent color for up and down reflines #14134

Merged
merged 2 commits into from May 27, 2019

Conversation

deepakchethan
Copy link
Contributor

@deepakchethan deepakchethan commented May 22, 2019

Fixes #1328

TODO

  • UTF support

After change:

Screen Shot 2019-05-26 at 7 49 30 PM

@lgtm-com
Copy link

lgtm-com bot commented May 22, 2019

This pull request introduces 1 alert when merging 36a1aec into 3e3d97b - view on LGTM.com

new alerts:

  • 1 for Returning stack-allocated memory

Comment posted by LGTM.com

libr/core/disasm.c Outdated Show resolved Hide resolved
libr/core/disasm.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@radare radare left a comment

Choose a reason for hiding this comment

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

.

@lgtm-com
Copy link

lgtm-com bot commented May 22, 2019

This pull request introduces 1 alert when merging 7b4946c1cc27c8cd704e4ab4b23e4a2bbc66340b into 680340c - view on LGTM.com

new alerts:

  • 1 for Returning stack-allocated memory

Comment posted by LGTM.com

@deepakchethan deepakchethan requested a review from radare May 26, 2019 14:20
@deepakchethan deepakchethan changed the title WIP WIP: different color for up and down reflines Dfferent color for up and down reflines May 26, 2019
@deepakchethan
Copy link
Contributor Author

deepakchethan commented May 26, 2019

I fixed the returning stack-allocated memory issue as well

Most of the fails in the Travis are due to the fact that previously empty strings were also placed between color and reset.
The current implementation ignores colors for spaces/ empty strings.
Should I update the tests?
Screen Shot 2019-05-26 at 7 45 05 PM

@radare
Copy link
Collaborator

radare commented May 26, 2019

Yes, please update the tests. lgtm

libr/anal/reflines.c Outdated Show resolved Hide resolved
libr/anal/reflines.c Outdated Show resolved Hide resolved
libr/core/disasm.c Outdated Show resolved Hide resolved
libr/core/disasm.c Outdated Show resolved Hide resolved
libr/core/disasm.c Outdated Show resolved Hide resolved
libr/core/disasm.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@radare radare left a comment

Choose a reason for hiding this comment

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

.

Added support for utf8

Changed colors

Change condition
@codecov-io
Copy link

Codecov Report

Merging #14134 into master will increase coverage by <.01%.
The diff coverage is 86.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14134      +/-   ##
==========================================
+ Coverage    37.8%   37.81%   +<.01%     
==========================================
  Files         949      949              
  Lines      305172   305236      +64     
==========================================
+ Hits       115367   115419      +52     
- Misses     189805   189817      +12
Impacted Files Coverage Δ
libr/core/disasm.c 74.92% <82.6%> (+0.05%) ⬆️
libr/anal/reflines.c 65.82% <93.93%> (+1.48%) ⬆️
libr/util/str.c 60.9% <0%> (-0.05%) ⬇️

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 27347ef...54b13ac. Read the comment docs.

@radare radare merged commit ffa473d into radareorg:master May 27, 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.

Difference colours for up-going and down-going arrows
3 participants