Add private bitflags #38

Merged
Amanieu merged 1 commit from private_bitflags into master 2016-01-22 17:31:11 -06:00
Amanieu commented 2016-01-16 04:00:58 -06:00 (Migrated from github.com)

This adds the ability to define private bitflags that aren't exported outside of the current module using priv flags.

Ideally this would be done by matching the existing behavior in the language: making private bitflags the default behavior and requiring the use of pub flags to export it outside the mode. However there are a large number of crates that use bitflags with a * version, which makes me somewhat nervous to break them all.

Fixes #25

This adds the ability to define private bitflags that aren't exported outside of the current module using `priv flags`. Ideally this would be done by matching the existing behavior in the language: making private bitflags the default behavior and requiring the use of `pub flags` to export it outside the mode. However there are a [large number](https://crates.io/crates/bitflags/reverse_dependencies) of crates that use `bitflags` with a `*` version, which makes me somewhat nervous to break them all. Fixes #25
alexcrichton commented 2016-01-19 14:56:56 -06:00 (Migrated from github.com)

I agree that private is the better default (e.g. matches Rust) and requiring an explicit pub is the best way to go here, so I'd personally prefer to go in that direction.

I agree that private is the better default (e.g. matches Rust) and requiring an explicit `pub` is the best way to go here, so I'd personally prefer to go in that direction.
Amanieu commented 2016-01-19 20:24:02 -06:00 (Migrated from github.com)

I changed it to private by default and requiring an explicit pub. However I am still worried that this will break a lot of crates that use bitflags with a * version: https://crates.io/crates/bitflags/reverse_dependencies

I changed it to private by default and requiring an explicit `pub`. However I am still worried that this will break a lot of crates that use `bitflags` with a `*` version: https://crates.io/crates/bitflags/reverse_dependencies
alexcrichton commented 2016-01-22 17:31:10 -06:00 (Migrated from github.com)

Ah yeah it's true that this may break some crates, but * dependencies are no longer allowed on crates.io and those are ones that probably need to be updated anyway. I wouldn't necessarily want to be locked in to prevent any breaking changes so it seems fine to do this.

Thanks again @Amanieu!

Ah yeah it's true that this may break some crates, but `*` dependencies are no longer allowed on crates.io and those are ones that probably need to be updated anyway. I wouldn't necessarily want to be locked in to prevent _any_ breaking changes so it seems fine to do this. Thanks again @Amanieu!
gkoz commented 2016-02-28 06:31:32 -06:00 (Migrated from github.com)

There's a confusing situation now: 0.4.0 from git master and the documentation at doc.rlo have default private but the latest crates.io 0.4.0 package has default public.

There's a confusing situation now: 0.4.0 from git master and the documentation at `doc.rlo` have default private but the latest crates.io 0.4.0 package has default public.
Amanieu commented 2016-02-28 11:05:13 -06:00 (Migrated from github.com)

This is because this PR hasn't been pushed to crates.io yet.

On Sun, Feb 28, 2016 at 12:31 PM, Gleb Kozyrev notifications@github.com
wrote:

There's a confusing situation now: 0.4.0 from git master and the
documentation at doc.rlo have default private but the latest crates.io
0.4.0 package has default public.

—
Reply to this email directly or view it on GitHub
https://github.com/rust-lang-nursery/bitflags/pull/38#issuecomment-189858146
.

This is because this PR hasn't been pushed to crates.io yet. On Sun, Feb 28, 2016 at 12:31 PM, Gleb Kozyrev notifications@github.com wrote: > There's a confusing situation now: 0.4.0 from git master and the > documentation at doc.rlo have default private but the latest crates.io > 0.4.0 package has default public. > > — > Reply to this email directly or view it on GitHub > https://github.com/rust-lang-nursery/bitflags/pull/38#issuecomment-189858146 > .
alexcrichton commented 2016-02-28 12:51:48 -06:00 (Migrated from github.com)

I've published 0.5.0 now with this change.

I've published 0.5.0 now with this change.
xitep commented 2016-03-01 16:08:42 -06:00 (Migrated from github.com)

hello, it looks like after this change bitflags are forced to standard types, e.g. u32. is this an accident or purpose?

the lmdb-rs project - which i tried to upgrade - uses amongst others libc::c_uint in its bitflags definitions. but i see no possibility to include the corresponding use libc::c_uint declaration in the newly generated dummy module.

any advice for me?

hello, it looks like after this change bitflags are _forced_ to standard types, e.g. u32. is this an accident or purpose? the `lmdb-rs` project - which i tried to upgrade - uses amongst others [libc::c_uint in its bitflags definitions.](https://github.com/vhbit/lmdb-rs/blob/master/src/core.rs#L183) but i see no possibility to include the corresponding `use libc::c_uint` declaration in the newly generated `dummy` module. any advice for me?
Amanieu commented 2016-03-01 16:57:18 -06:00 (Migrated from github.com)

@xitep I think the best advice would be to just use u32 for now. libc::c_uint is going to be equivalent to that type on most sane platforms anyways.

@alexcrichton I tried fixing this but couldn't figure out a way to reliably pass the type down to the dummy module (type doesn't seem to be covered by macro hygiene). Note that the associated constants version (#24) doesn't have this issue since it doesn't use dummy modules.

@xitep I think the best advice would be to just use `u32` for now. `libc::c_uint` is going to be equivalent to that type on most sane platforms anyways. @alexcrichton I tried fixing this but couldn't figure out a way to reliably pass the type down to the dummy module (`type` doesn't seem to be covered by macro hygiene). Note that the associated constants version (#24) doesn't have this issue since it doesn't use dummy modules.
alexcrichton commented 2016-03-01 17:17:19 -06:00 (Migrated from github.com)

Can't you use an absolute path like ::libc::c_uint?

Can't you use an absolute path like `::libc::c_uint`?
xitep commented 2016-03-02 01:05:45 -06:00 (Migrated from github.com)

@alexcrichton thank you! yes, absolute references make it!

is this the long term plan or do we plan for another solution?

@alexcrichton thank you! yes, absolute references make it! is this the long term plan or do we plan for another solution?
alexcrichton commented 2016-03-02 12:32:45 -06:00 (Migrated from github.com)

Probably, this library is in a relatively bad state unfortunately. Can you open an issue on this?

Probably, this library is in a relatively bad state unfortunately. Can you open an issue on this?
Sign in to join this conversation.
No reviewers
No labels
bug
good first issue
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-bitflags/bitflags!38
No description provided.