Skip to content

Comments

Add ruler to histogram output#4776

Closed
DimitrisVita wants to merge 1 commit intorizinorg:devfrom
DimitrisVita:enhance-histogram-y-axis-numbering
Closed

Add ruler to histogram output#4776
DimitrisVita wants to merge 1 commit intorizinorg:devfrom
DimitrisVita:enhance-histogram-y-axis-numbering

Conversation

@DimitrisVita
Copy link

@DimitrisVita DimitrisVita commented Dec 17, 2024

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

This pull request adds a vertical ruler to the horizontal histogram output in librz/cons/histogram.c. The ruler includes value ranges and uses a vertical Unicode line character if scr.utf8=true.

I've added y-axis numbering, which is computed as 255 - threshold, where the threshold is calculated for each row. Please let me know if this approach is acceptable or if any adjustments are needed. Should it be added to the interactive histogram too?

Here are two images showing the expected output:

  • Image 1: p== 50 with scr.utf8=true

peqeq50_true

  • Image 2: p== 50 with scr.utf8=false

peqeq50_false

Test plan

  1. Enable scr.utf8=true in the configuration.
  2. Generate a horizontal histogram using the p== command.
  3. Verify that the vertical ruler is displayed correctly with value ranges.
  4. Disable scr.utf8=true and repeat the test to ensure the ruler is displayed correctly without Unicode characters.

Closing issues

Addresses #129 but might need improvement

@DimitrisVita DimitrisVita marked this pull request as draft December 17, 2024 12:16
@DimitrisVita DimitrisVita marked this pull request as ready for review December 17, 2024 15:06
@XVilka
Copy link
Member

XVilka commented Dec 17, 2024

In such PRs (related to UI) please attach screenshots.

@XVilka
Copy link
Member

XVilka commented Dec 18, 2024

@DimitrisVita I think such ruler should be moved to the left, not the right. Or this can be one more histogram option

Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

I also think, that while vertical ruler should be added to horizontal histogram, horitzontal ruler should be added to vertical histogram.

Rot127
Rot127 previously approved these changes Dec 18, 2024
@DimitrisVita DimitrisVita force-pushed the enhance-histogram-y-axis-numbering branch from b45d2c6 to ab9a6cf Compare December 20, 2024 18:54
@XVilka
Copy link
Member

XVilka commented Jan 4, 2025

@DimitrisVita hi, have you had some time to continue working on this?

@notxvilka
Copy link
Contributor

@DimitrisVita if you have any questions, don't hesitate to ask them on the Rizin Dev channel on our Mattermost.

@notxvilka notxvilka modified the milestones: 0.8.0, 0.9.0 Mar 2, 2025
@notxvilka
Copy link
Contributor

Closing this because no activity and reaction

@notxvilka notxvilka closed this May 11, 2025
@cer-0 cer-0 mentioned this pull request Jul 26, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RzCons UX/UI User Interface/User experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants