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

Remove bitflags dependency #1440

Closed
wants to merge 1 commit into from
Closed

Conversation

npmccallum
Copy link

The only real use of bitflags was to save a tiny bit of memory. It isn't
exposed externally and we can just use a flag enumeration combined with
a HashSet to replace it. Because of this, let's not subject this
dependency to all projects which depend on clap.

The only real use of bitflags was to save a tiny bit of memory. It isn't
exposed externally and we can just use a flag enumeration combined with
a HashSet to replace it. Because of this, let's not subject this
dependency to all projects which depend on clap.
@npmccallum
Copy link
Author

This PR is an adaptation of #1436 to the v3 branch.

@npmccallum
Copy link
Author

@spacekookie The test failures are unrelated.

@kbknapp
Copy link
Member

kbknapp commented Apr 5, 2019

@npmccallum I'm going to close this as per the discussion in #1436 however I'm 100% open to a local bitflags like solution against the v3-master branch.

All we really need out of a bit flags style solutions is:

  • the ability to map an enum to one or more bits of the field, bonus if the width can vary using macros. However it's fine we just pick an arbitrary width like 64bit
  • the ability to set/unset bits
  • the ability to check if a particular bit(s) is set or not

@kbknapp kbknapp closed this Apr 5, 2019
@BurntSushi
Copy link
Contributor

@kbknapp I did something super simple with a tiny macro for regex-syntax: https://github.com/rust-lang/regex/blob/9687986de45378c744180cc696286aad7db83c5e/regex-syntax/src/hir/mod.rs#L1425-L1473 --- Not sure if it works for clap, though, it might!

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.

3 participants