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

fix: support ints in cast #584

Merged
merged 1 commit into from
Jan 29, 2022
Merged

fix: support ints in cast #584

merged 1 commit into from
Jan 29, 2022

Conversation

mds1
Copy link
Collaborator

@mds1 mds1 commented Jan 26, 2022

Currently this just modifies the clap config to allow hyphens for commands that can take negative values as input args. A sanity check that I didn't miss any commands would be appreciated.

Clap also has an option to allow hyphens globally, but only allowing them as-needed is preferable.

The commands with the "negative values not yet supported" comment still don't work due to ints not being supported somewhere in the underlying methods. Will get that fixed next before taking this out of draft

@onbjerg onbjerg added the T-bug Type: bug label Jan 26, 2022
@mds1 mds1 marked this pull request as ready for review January 28, 2022 15:14
@mds1
Copy link
Collaborator Author

mds1 commented Jan 28, 2022

Taking this out of draft, and figured we could merge as-is and finish adding int support in a separate PR. In it's current form:

  • This PR fixes cast abi-encode, which now works for both ints and uints
  • The other modified cast methods still fail, like they did before. But instead of failing at arg parsing they fail internally

It'll be a bit until I find time to get the rest of the int support working, so figured it's not worth holding up the fix for cast abi-encode in the meantime

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm,
could you create a followup issue with what we need to fix internally now that we can parse them correctly at the arg parse level?

@mattsse mattsse merged commit 1f27b2f into foundry-rs:master Jan 29, 2022
@mds1 mds1 mentioned this pull request Jan 29, 2022
@mds1
Copy link
Collaborator Author

mds1 commented Jan 29, 2022

@mattsse just created gakonst/ethers-rs#842 to track a useful ethers-rs improvement which would make signed int support simpler. Once that's done, I've documented what we'd need to do next in #29 (comment), where I'm tracking various unsupported cast features to avoid clutter from too many issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants