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.

Sign up for GitHub

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

Jump to bottom

Add environment variables documentation to manpages and -hh #4560

Merged
merged 12 commits into from
Jul 10, 2024
Merged

Add environment variables documentation to manpages and -hh #4560

merged 12 commits into from
Jul 10, 2024

Conversation

satk0
Copy link
Contributor

@satk0 satk0 commented Jul 6, 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

Part of solving the issue #4445. The only thing left is the book.

There is no usage of the RZ_LOGTRAPLEVEL environment variable in the code so it is impossible to document it.

Test plan

...

Closing issues

...

@github-actions github-actions bot added documentation Improvements or additions to documentation rizin rz-asm rz-bin labels Jul 6, 2024
XVilka
XVilka requested changes Jul 7, 2024
View reviewed changes
librz/main/rz-bin.c Outdated Show resolved Hide resolved
librz/main/rz-bin.c Outdated Show resolved Hide resolved
librz/main/rz-bin.c Outdated Show resolved Hide resolved
librz/main/rz-asm.c Outdated Show resolved Hide resolved
librz/main/rz-asm.c Outdated Show resolved Hide resolved
librz/main/rizin.c Outdated Show resolved Hide resolved
librz/main/rizin.c Outdated
" RZ_DEBUG_TOOL=gdb debug tool to use when showing error messages and crash signal\n"
" RZ_DYLDCACHE_FILTER dyld cache filter (MacOS dynamic libraries location(s) at runtime)\n"
" RZ_HTTP_AUTHFILE HTTP Authentification user file\n"
" RZ_LOGCOLORS should the log output use colors (TODO)\n"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe remove since it's not implemented

Copy link
Member

@wargio wargio Jul 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Log colors has been implemented by me https://github.com/rizinorg/rizin/blob/dev/librz/util/log.c#L33 (3033f62)

@wargio
Copy link
Member

wargio commented Jul 7, 2024

I think i have removed RZ_LOGTRAPLEVEL and replaced it with RZ_ABORTLEVEL

wargio
wargio reviewed Jul 7, 2024
View reviewed changes
binrz/man/rizin.1 Outdated
Comment on lines 167 to 173
.Pp
TEMP: Temp directory path (Windows)
.Pp
TMPDIR: tmp directory path (eg: /tmp)
.Pp
WT_SESSION: check if in Windows terminal
.Pp
Copy link
Member

Choose a reason for hiding this comment

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

probably these, ALACRITTY_LOG and PATH should not be in the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

@XVilka

@satk0
Copy link
Contributor Author

satk0 commented Jul 7, 2024

Thanks for such a fast feedback. I was asleep, sorry. So I would try to document RZ_ABORTLEVEL instead of RZ_LOGTRAPLEVEL and make changes as @XVilka suggested.

@satk0
Copy link
Contributor Author

satk0 commented Jul 7, 2024

Done, let me know if there is anything more I can add/change.

wargio
wargio reviewed Jul 7, 2024
View reviewed changes
librz/main/rizin.c Outdated
" SFLIBPATH SFLib syscall library path\n"
" SHELL=sh shell to use (eg: \"sh\")\n"
" TEMP Temp directory path (Windows)\n"
" TMPDIR=/tmp tmp directory path (eg: /tmp)\n"
Copy link
Member

@wargio wargio Jul 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
" TMPDIR=/tmp tmp directory path (eg: /tmp)\n"
" TMPDIR tmp directory path (eg: /tmp)\n"

wargio
wargio reviewed Jul 7, 2024
View reviewed changes
librz/main/rizin.c Outdated Show resolved Hide resolved
wargio
wargio reviewed Jul 7, 2024
View reviewed changes
librz/main/rz-bin.c Outdated Show resolved Hide resolved
@satk0
Copy link
Contributor Author

satk0 commented Jul 7, 2024 •
edited
Loading

Done, I deleted default values for RZ_DEBUG_ASSERT=1 and RZ_DEBUG_TOOL=gdb too

XVilka
XVilka requested changes Jul 8, 2024
View reviewed changes
binrz/man/rizin.1 Outdated Show resolved Hide resolved
@satk0
Copy link
Contributor Author

satk0 commented Jul 8, 2024 •
edited
Loading

I really hate whitespaces, btw I done it

XVilka
XVilka requested changes Jul 9, 2024
View reviewed changes
binrz/man/rizin.1 Outdated Show resolved Hide resolved
librz/main/rizin.c Outdated Show resolved Hide resolved
@XVilka XVilka requested a review from wargio July 9, 2024 14:51
@satk0
Copy link
Contributor Author

satk0 commented Jul 9, 2024

Done, let me know if there is anything more I can help

@wargio wargio linked an issue Jul 10, 2024 that may be closed by this pull request
Describe and map in documentation all the environment variables #4445
Open
@wargio wargio merged commit e3d27c9 into rizinorg:dev Jul 10, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XVilka XVilka XVilka approved these changes

@wargio wargio wargio approved these changes

@kazarmy kazarmy Awaiting requested review from kazarmy kazarmy is a code owner

@thestr4ng3r thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r is a code owner

@ret2libc ret2libc Awaiting requested review from ret2libc ret2libc is a code owner

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation rizin rz-asm rz-bin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Describe and map in documentation all the environment variables
3 participants
@satk0 @wargio @XVilka