Closed hacknus closed 1 month ago
This seems to be an accidentally forced dependency on the "serde" feature. Enabling it resolves the issue, but it's supposed to be optional.
Looks like the current table implementation just assumes that the serde feature is enabled. There are unconditional calls to TableState::load
for example in TableBuilder::header
. And TableState::load
unconditionally tries to call get_persisted
.
Workaround until this is fixed:
egui_extras = { version = "0.28.0", features = ["serde"] }
Adding this to Cargo.toml
even works if it's not a direct dependency (required by egui_commonmark
for example).
We should:
A) set up a CI step to catch this problem
B) only call get_persisted
if the serde
feature is enabled
The problem is actually in egui, not egui_extras. It happens if we try to build egui_extras without the serde
feature while something else enabled the persistence
feature in egui. This was previously not possible because serde was a mandatory dependency in egui_extras.
cargo build -p egui_extras --features egui/persistence
Enabling the feature causes the trait bounds to change in a semver breaking way, which should not happen because features are supposed to be additive. I don't know if there is a way to avoid this, though, aside from making serde mandatory in egui.
Good catch… yeah, if persistence
is on for egui
and serde
is off for egui_extras
, we will have this problem. Not easily solved if we want serde
to be optional for egui_extras
.
As a short-term solution I can make serde
a default (opt-out) feature of egui_extras
, so at least fewer people get bitten by this.
Good catch… yeah, if persistence is on for egui and serde is off for egui_extras, we will have this problem. As a short-term solution I can make serde a default (opt-out) feature of egui_extras, so at least fewer people get bitten by this.
Wouldn't making the persistence
feature depend on egui_extras/serde
also have worked? :no_mouth: It would have created a bit less headache IMHO, and maybe it could even have been considered a "proper" solution, not just a short term one? :thinking: Of course it's entirely possible that I'm just missing something that makes this not quite as simple...
egui/persistence
cannot depend on egui_extras/serde
, because egui_extras
depends on egui
Ah, right, this is what I missed...
We should be able to fix this by only calling get_persisted/insert_persisted
if #[cfg(feature = "serde")]
, and otherwise call get_temp/insert_temp
Is anyone working on this? I'm eagerly anticipating the new release.
We should be able to fix this by only calling
get_persisted/insert_persisted
if#[cfg(feature = "serde")]
, and otherwise callget_temp/insert_temp
If this is what has to be done anyway, then maybe get_persisted/insert_persisted
in egui should be conditional on the persistence
feature instead of having different trait bounds? That means each crate that uses egui would have to implement its own logic to call get_persisted
or get_temp
depending on its own enabled features.
This would be in accordance with the recommendation on https://doc.rust-lang.org/cargo/reference/features.html#feature-unification
A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features. A feature should not introduce a SemVer-incompatible change.
vergen recently made that change.
https://github.com/rustyhorde/vergen?tab=readme-ov-file#%EF%B8%8F-notes-on-version-9-%EF%B8%8F
If this is what has to be done anyway, then maybe
get_persisted/insert_persisted
in egui should be conditional on thepersistence
feature instead of having different trait bounds? That means each crate that uses egui would have to implement its own logic to callget_persisted
orget_temp
depending on its own enabled features.
The issue at hand occurs when egui:persistence
is ON and egui_extras:serde
is OFF.
Oh, was this the last open issue for the "Next Patch Release" milestone? :eyes: ^^
I tried the new version of egui (0.28) but egui-extras seems to fail compiling with the following messages:
in `synthax_highlighting.rs
and in
table.rs
:My
cargo.toml
looks like this:And I'm using it on macOS 14.5.
egui-phosphor is bumped to egui 0.28 and works fine. the issue lies in egui_extras.