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

Update tools help and man pages #4145

Merged
merged 29 commits into from
Jan 31, 2024
Merged

Update tools help and man pages #4145

merged 29 commits into from
Jan 31, 2024

Conversation

byteninjaa0
Copy link
Contributor

@byteninjaa0 byteninjaa0 commented Jan 24, 2024

SQUASH ME

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

Update man pages to reflect the current state of Rizin tools.

Test plan

  • CI is green
  • See if changes make sense

Closing issues

Closes #3731

binrz/man/rz-bin.1 Outdated Show resolved Hide resolved
binrz/man/rz-run.1 Outdated Show resolved Hide resolved
Copy link
Member

@wargio wargio left a comment

Choose a reason for hiding this comment

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

wow nice changes!!

binrz/man/rz-asm.1 Outdated Show resolved Hide resolved
binrz/man/rz-asm.1 Outdated Show resolved Hide resolved
binrz/man/rz-asm.1 Outdated Show resolved Hide resolved
binrz/man/rz-asm.1 Outdated Show resolved Hide resolved
binrz/man/rz-asm.1 Outdated Show resolved Hide resolved
binrz/man/rz-bin.1 Outdated Show resolved Hide resolved
binrz/man/rz-find.1 Outdated Show resolved Hide resolved
binrz/man/rz-run.1 Show resolved Hide resolved
binrz/man/rz-test.1 Outdated Show resolved Hide resolved
binrz/man/rz-test.1 Outdated Show resolved Hide resolved
@XVilka XVilka added this to the 0.7.0 milestone Jan 24, 2024
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.

Overall, it looks better now. As something might have slipped my review, let more eyes check first before merging.

binrz/man/rz-bin.1 Outdated Show resolved Hide resolved
binrz/man/rz-bin.1 Outdated Show resolved Hide resolved
binrz/man/rz-run.1 Show resolved Hide resolved
@byteninjaa0
Copy link
Contributor Author

byteninjaa0 commented Jan 25, 2024

i am reverting the man rz-run as nothing has been changed all the directives and option is working as expected,
if any change has to be done do let me know

binrz/man/rz-asm.1 Outdated Show resolved Hide resolved
binrz/man/rz-asm.1 Outdated Show resolved Hide resolved
binrz/man/rz-asm.1 Outdated Show resolved Hide resolved
binrz/man/rz-ax.1 Outdated Show resolved Hide resolved
binrz/man/rz-ax.1 Outdated Show resolved Hide resolved
binrz/man/rz-ax.1 Outdated Show resolved Hide resolved
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.

Looks much better now! Few things left.

binrz/man/rz-ax.1 Outdated Show resolved Hide resolved
binrz/man/rz-bin.1 Outdated Show resolved Hide resolved
binrz/man/rz-bin.1 Outdated Show resolved Hide resolved
binrz/man/rz-bin.1 Outdated Show resolved Hide resolved
binrz/man/rz-bin.1 Outdated Show resolved Hide resolved
binrz/man/rz-hash.1 Outdated Show resolved Hide resolved
@@ -1,60 +1,64 @@
.Dd Mar 31, 2020
.Dd Jan 24, 2024
.Dt RZ_TEST 1
.Sh NAME
.Nm rz-test
.Nd rizin regression testsuite
Copy link
Member

Choose a reason for hiding this comment

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

Not just testsuite but something like testsuite running tool. @wargio @kazarmy do you have better ideas for the description?

.It Fl v
Show version number
.It Fl q
Quiet.
.It Fl V
Copy link
Member

Choose a reason for hiding this comment

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

Sounds like an order. Quiet mode instead. Also remove the period.

binrz/man/rz-test.1 Outdated Show resolved Hide resolved
librz/main/rz-gg.c Outdated Show resolved Hide resolved
@XVilka XVilka added documentation Improvements or additions to documentation high-priority labels Jan 27, 2024
@XVilka
Copy link
Member

XVilka commented Jan 27, 2024

  1. Please rebase
  2. Could you please also add the man pages directory into the documentation key in .github/labeler.yml? So that it will mark any changes with the documentation label automatically.

@XVilka XVilka added the blocker label Jan 27, 2024
binrz/man/rz-run.1 Outdated Show resolved Hide resolved
@wargio
Copy link
Member

wargio commented Jan 27, 2024

all these changes should be applied also to the actual -h help message of each utility which required these changes.

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 think it's already a big improvement and can be merged as is. If there is something else left, we can fix it in other PRs.

@XVilka XVilka changed the title Updating man Updating man pages Jan 29, 2024
binrz/rz-test/rz-test.c Outdated Show resolved Hide resolved
@wargio
Copy link
Member

wargio commented Jan 29, 2024

Please fix verbs conjugation in the man pages PR. Some verbs are simple while others use third person.
They all should be simple - Show not Shows, etc.. within the C files.

@XVilka
Copy link
Member

XVilka commented Jan 29, 2024

@byteninjaa0, please make those verb conjugation adjustments not only in C files - I can see there are a lot of inconsistencies in the man pages themselves, too.

@byteninjaa0
Copy link
Contributor Author

this too ?
image

@wargio
Copy link
Member

wargio commented Jan 31, 2024

yes those should appear in the man pages too

@byteninjaa0
Copy link
Contributor Author

byteninjaa0 commented Jan 31, 2024

i think that the summary/description of the utilities should be in third person otherwise grammatically it would be wrong, isn't it?

@XVilka XVilka changed the title Updating man pages Update tools help and man pages Jan 31, 2024
@XVilka XVilka merged commit 8cfd11b into rizinorg:dev Jan 31, 2024
35 checks passed
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.

Update man pages for Rizin tools
4 participants