Simplified conditional compilation #138
Conversation
| use crate::windows_native::utils::PeakIterExt; | ||
| use super::typedefs::{Caps, LinkCollectionNode}; | ||
| use super::types::{ItemNodeType, Items, MainItemNode, MainItems}; | ||
| use super::super::error::{WinError, WinResult}; |
There was a problem hiding this comment.
I am not really a fan of relative paths for imports, when its necessary to go up multiple levels.
Its much clearer with one glance from where the item was imported with the absolute path.
There was a problem hiding this comment.
I made this change to keep the backend more self-contained and not depend on the name it was imported under. My original idea was to do something like this:
#[cfg_attr(feature = "linux-native", path = "linux_native.rs")]
#[cfg_attr(feature = "windows-native", path = "windows_native/mod.rs")]
mod platform;Which obviously broke those imports.
After changing my approach to cfg_if I decided to nevertheless keep it. I can revert the paths if you want.
| use windows_native::HidApiBackend; | ||
| cfg_if! { | ||
| if #[cfg(all(feature = "linux-native", target_os = "linux"))] { | ||
| //#[cfg_attr(docsrs, doc(cfg(all(feature = "linux-native", target_os = "linux"))))] |
There was a problem hiding this comment.
Why comment out these cfg_attrs, and not the ones below?
There was a problem hiding this comment.
The attribute is used to document that a certain function is limited to certain configurations:
The native backends should not appear to the public api and therefore the attribute seemed pretty useless. However I just commented them out for now to make it easier to reenable them in case I am wrong and they did actually serve a purpose that I missed.
There was a problem hiding this comment.
Yeah I think you are right. We can just remove them completely, since its backend code.
|
My last concern was, that cfg_if completely breaks IDE support. Apparently its now limited to only being able to index one of multiple cfg_if branches, which should be good enough. |
I recently fount out that this crate currently fails to compile on non-windows platforms when the
windows-nativefeature is active. This is caused by a missingtarget_os = "windows"in one of the#[cfg(...)]attributes.I refactored the individual
#[cfg(...)]attributes to use thecfg_ifmacro instead to improve readablillity and hopefully prevent such mistakes in the future.