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

RzTable API Refactoring #4625

Merged
merged 2 commits into from
Sep 18, 2024
Merged

Conversation

brightprogrammer
Copy link
Contributor

@brightprogrammer brightprogrammer commented Sep 9, 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
RzTable API refactoring.

TODOs for this PR :

  • Provide the API to set a particular value given the names of the column and the row (overwrite if exists, adds if not)
  • Provide the API to set a particular value given the indexes of the column and the row (overwrite if exists, adds if not)
  • Provide the API to add a particular value given the names of the column and the row without overwriting
  • Provide the API to add a particular value given the indexes of the column and the row without overwriting
  • Provide the API to form a table/row/column given the HtPP/HtUP/HtUU as an argument
  • Switch the internal storage of the RzTable from RzList to either vectors or hashtable (HtPP or something like that) : This is already addressed by Refactor RzTable: change the internal storage to RzVector #2421 and Refactor RzTable: change the implementation to RzVector #2536
  • Implement tests for these and other missing APIs

This PR also adds a new function to RzTable API for adding a new row using a format string and a va_list.

Test plan
N/A

Closing issues
#1961

@XVilka
Copy link
Member

XVilka commented Sep 17, 2024

@brightprogrammer instead of waiting when all of the issue is covered, I recommend just doing it piece-by-piece, and merge this as is. Please rebase and mark as ready for the review.

@brightprogrammer
Copy link
Contributor Author

Ok

For cases when we need to directly use arguments of a variadic function
to add a new row to `RzTable`, the new `rz_table_add_vrowf` will take a
`va_list` and a format string to do the same.
@brightprogrammer
Copy link
Contributor Author

Running sys/clang-format.py changes lots of files.

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.

Please fix linter and LGTM.

@wargio
Copy link
Member

wargio commented Sep 18, 2024

Running sys/clang-format.py changes lots of files.

ensure you run clang-format16 (you can supply the clang format bin path by cmd line to the python script)

@XVilka XVilka merged commit 5bfcbbf into rizinorg:dev Sep 18, 2024
44 checks passed
brightprogrammer added a commit to RevEngAI/reai-rz that referenced this pull request Sep 18, 2024
The commit refactors and fixes the table generation bugs in
`ReaiPluginTable` abstraction. The plugin now completely uses the API
provided by `RzTable` to generate the table and display it.

This was possible because of recent merge of PR
rizinorg/rizin#4625

This also means that users might face some problems if they use a
release version of Rizin/Cutter. Users (for now) must build Rizin from
source until unless a new version of Rizin is released with the changes
in PR present.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants