Skip to content

Comments

Remove old shell#5004

Merged
notxvilka merged 7 commits intorizinorg:devfrom
Rot127:remove_old_shell
Mar 17, 2025
Merged

Remove old shell#5004
notxvilka merged 7 commits intorizinorg:devfrom
Rot127:remove_old_shell

Conversation

@Rot127
Copy link
Member

@Rot127 Rot127 commented Mar 16, 2025

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 every RZ_API function and struct this PR changes.
  • I've added tests that prove my changes are effective (required for changes to RZ_API).
  • I've updated the Rizin book with the relevant information (if needed).

Detailed description

Hopefully this is all.

Test plan

all green

Closing issues

...

Rot127 added 2 commits March 16, 2025 11:39
During the tests the global RzCons is accessed. Because it was
never initialized all members were 0.
Leading to undefined behavior and breaking the test with ASAN enabled.
@github-actions github-actions bot added documentation Improvements or additions to documentation rz-test API RzCore labels Mar 16, 2025
@Rot127 Rot127 force-pushed the remove_old_shell branch 3 times, most recently from 3d10817 to 4e8afad Compare March 16, 2025 17:20
@Rot127 Rot127 force-pushed the remove_old_shell branch from 4e8afad to 140599e Compare March 16, 2025 17:22
@Rot127 Rot127 added this to the 0.8.0 milestone Mar 16, 2025
Copy link
Contributor

@notxvilka notxvilka left a comment

Choose a reason for hiding this comment

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

Good riddance!

@notxvilka
Copy link
Contributor

notxvilka commented Mar 16, 2025

I think there was more code to remove though: rz_core_cmd_subst_i() and rz_core_cmd_subst(), for example

@Rot127
Copy link
Member Author

Rot127 commented Mar 16, 2025

I think there was more code to remove though: rz_core_cmd_subst_i() and rz_core_cmd_subst(), for example

Mh, yeah. Seems like it. I just checked for oldinput and was hoping the rest follows. Let me check again.

@Rot127 Rot127 marked this pull request as draft March 16, 2025 18:41
@Rot127 Rot127 marked this pull request as ready for review March 16, 2025 19:11
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.

i think there is one more old command aeH which calls help_detail_ae

@Rot127
Copy link
Member Author

Rot127 commented Mar 17, 2025

i think there is one more old command aeH which calls help_detail_ae

Yes, I kept the aeH command and help. Because it gets removed anyways with RzIL. But also didn't want to transfer the whole thing into the yaml file, if we throw it away anyways.
Can do it though if you see an immediate advantage.

@Rot127 Rot127 force-pushed the remove_old_shell branch from 7e6783f to d103e2a Compare March 17, 2025 12:16
@notxvilka notxvilka requested a review from wargio March 17, 2025 14:20
@notxvilka
Copy link
Contributor

i think there is one more old command aeH which calls help_detail_ae

Yes, I kept the aeH command and help. Because it gets removed anyways with RzIL. But also didn't want to transfer the whole thing into the yaml file, if we throw it away anyways. Can do it though if you see an immediate advantage.

Agree, that is reasonable to postpone its removal to the ESIL removal.

@notxvilka notxvilka merged commit 1b50a5b into rizinorg:dev Mar 17, 2025
46 checks passed
@Rot127 Rot127 deleted the remove_old_shell branch March 17, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API documentation Improvements or additions to documentation rz-test RzCore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants