iced icon indicating copy to clipboard operation
iced copied to clipboard

Added convenience functions for window::icon::Icon

Open daladim opened this issue 4 years ago • 5 comments

See #1170

Since the image dependency was already an (optional) dependency of iced, why not provide these convenience functions that save a few lines of code in applications that need an icon

daladim avatar Dec 28 '21 23:12 daladim

Thank you for the PR!

Could you make it so that your convenience function is only available if the iced image feature is enabled?

The image crate is quite large, and iced tries to give users the option for simplicity, but tries to be as light as it can be by default. This is the reason that image isn't already included by default.

13r0ck avatar Dec 29 '21 06:12 13r0ck

Well, I had already gated this behind an image_rs feature I had just added, for the very reasons you described. I've changed this PR so that this optional feature is now tied to the already-existing image feature.

Note that this pulls all the default features (= supported file formats) of image, there's no format-by-format opt-in as some other sub-crates of iced do.

I'm afraid I cannot remove the image_rs feature and keep only the image feature, since image is both the name of a crate and a feature. To do so, we would need:

  • either to rename the image feature so that it does not clash with the crate name
  • wait for https://github.com/rust-lang/cargo/issues/5565 to be ever stabilized
  • some other solution I would not be aware of?

daladim avatar Dec 29 '21 16:12 daladim

Also, I'm not sure how docs.rs generates its doc. Will these functions be shown in https://docs.rs/iced/latest/iced/?

iced's Cargo.toml already contains

[package.metadata.docs.rs]
rustdoc-args = ["--cfg", "docsrs"]
features = ["image", "svg", "canvas"]

...but I'm not sure how this will render the functions that are gated behind the image_rs feature instead of the image feature

daladim avatar Dec 29 '21 17:12 daladim

I did not manage to build the doc locally though the docs.rs project to test the image_rs-gated functions are visible, but since this feature is implied by the image one, it should be fine.

I think this MR is ready to be merged. What's your opinion on this? Thanks

daladim avatar Jan 19 '22 13:01 daladim

I'm finally having a green CI on this PR.

Is this PR OK for you? Or do you need anything else from me before merging it?

Thanks

daladim avatar Mar 01 '22 21:03 daladim