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

Implements #18 issue/feature, the "to _radix_" command: #19

Merged
merged 7 commits into from Nov 14, 2022

Conversation

mgaggero
Copy link
Contributor

Implements the "to radix" command to quickly and temporary convert a number or the result of an expression to a different radix (for example when working with memory address and segments sizes):

cork> 0xCAFE to dec
51966
cork> 0xCAFE + 2 * 8 * 0x20 to dec
52478
cork> 0xCAFE + 2 * 8 * 0x20
0xccfe
cork> ans to bin
0b1100110011111110
cork> 1234 to hex
0x4d2
cork> (0xCAFE + 2 * 8 * 0x20) to oct
0o146376

cork_to_radix_example

Copy link
Owner

@RedDocMD RedDocMD left a comment

Choose a reason for hiding this comment

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

Nice work!
I have made a few comments. Please look into them.
Also, I think you may need to run your code through rustfmt.

src/expression.peg Outdated Show resolved Hide resolved
src/expression.rs Outdated Show resolved Hide resolved
src/expression.rs Outdated Show resolved Hide resolved
src/expression.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@mgaggero mgaggero left a comment

Choose a reason for hiding this comment

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

Commit 8b94b9e implements the requested changes.

@RedDocMD
Copy link
Owner

RedDocMD commented Nov 8, 2022

Thank you for the changes!
I think a test or two would not be amiss. In particular, you can see a given string is actually parsed into a Command::Convert, and if invalid bases are rejected.

@mgaggero
Copy link
Contributor Author

mgaggero commented Nov 9, 2022

Thank you for the changes! I think a test or two would not be amiss. In particular, you can see a given string is actually parsed into a Command::Convert, and if invalid bases are rejected.

You're right, sure, I forgot the tests. Added in the last commit.

@RedDocMD
Copy link
Owner

Thanks a lot! I think I will merge it now, since you have covered all bases pretty nicely.

@RedDocMD RedDocMD merged commit bb0b42e into RedDocMD:main Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants