Skip to content

Commit

Permalink
Remove Env::default
Browse files Browse the repository at this point in the history
This was a bit of a footgun; it would always configure localization
resources, which doesn't make sense if we want to use an `Env` as just a
bag of key/values in some contexts.

Specifically, we may want to do this for #1658.

The fix is a two-parter:

- the `LocalizationManager` owned by the env is now optional, although
  it is expected to exist for the root env. The framework makes sure this
  is the case.
- There is a new `Env::empty` method that creates a new empty
  environment, with no localization resources. This is suitable for the
  'specifying overrides' case.
  • Loading branch information
cmyr committed Aug 4, 2021
1 parent dc17683 commit 5739d33
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 24 deletions.
2 changes: 1 addition & 1 deletion druid/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ impl<T: Data> AppLauncher<T> {
let mut env = self
.l10n_resources
.map(|it| Env::with_i10n(it.0, &it.1))
.unwrap_or_default();
.unwrap_or_else(Env::with_default_i10n);

if let Some(f) = self.env_setup.take() {
f(&mut env, &data);
Expand Down
2 changes: 1 addition & 1 deletion druid/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,7 @@ mod tests {
state: &mut state,
};

let env = Env::default();
let env = Env::with_default_i10n();

widget.lifecycle(&mut ctx, &LifeCycle::WidgetAdded, &1, &env);
assert!(ctx.widget_state.children.may_contain(&ID_1));
Expand Down
30 changes: 20 additions & 10 deletions druid/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub struct Env(Arc<EnvImpl>);
#[derive(Clone)]
struct EnvImpl {
map: HashMap<ArcStr, Value>,
l10n: Arc<L10nManager>,
l10n: Option<Arc<L10nManager>>,
}

/// A typed [`Env`] key.
Expand Down Expand Up @@ -228,7 +228,7 @@ impl Env {
///
/// ```no_run
/// # use druid::Env;
/// # let env = Env::default();
/// # let env = Env::empty();
/// # let widget_id = 0;
/// # let my_rect = druid::Rect::ZERO;
/// if env.get(Env::DEBUG_WIDGET) {
Expand Down Expand Up @@ -359,9 +359,11 @@ impl Env {
/// Returns a reference to the [`L10nManager`], which handles localization
/// resources.
///
/// This always exists on the base `Env` configured by druid.
///
/// [`L10nManager`]: struct.L10nManager.html
pub(crate) fn localization_manager(&self) -> &L10nManager {
&self.0.l10n
pub(crate) fn localization_manager(&self) -> Option<&L10nManager> {
self.0.l10n.as_deref()
}

/// Given an id, returns one of 18 distinct colors
Expand Down Expand Up @@ -504,18 +506,26 @@ static DEBUG_COLOR: &[Color] = &[
Color::rgb8(0, 0, 0),
];

impl Default for Env {
fn default() -> Self {
impl Env {
/// Returns an empty `Env`.
///
/// This is useful for creating a set of overrides.
pub fn empty() -> Self {
Env(Arc::new(EnvImpl {
l10n: None,
map: HashMap::new(),
}))
}

pub(crate) fn with_default_i10n() -> Self {
Env::with_i10n(vec!["builtin.ftl".into()], "./resources/i18n/")
}
}

impl Env {
pub(crate) fn with_i10n(resources: Vec<String>, base_dir: &str) -> Self {
let l10n = L10nManager::new(resources, base_dir);

let inner = EnvImpl {
l10n: Arc::new(l10n),
l10n: Some(Arc::new(l10n)),
map: HashMap::new(),
};

Expand Down Expand Up @@ -647,7 +657,7 @@ mod tests {
#[test]
fn string_key_or_value() {
const MY_KEY: Key<ArcStr> = Key::new("org.linebender.test.my-string-key");
let env = Env::default().adding(MY_KEY, "Owned");
let env = Env::empty().adding(MY_KEY, "Owned");
assert_eq!(env.get(MY_KEY).as_ref(), "Owned");

let key: KeyOrValue<ArcStr> = MY_KEY.into();
Expand Down
13 changes: 8 additions & 5 deletions druid/src/localization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,16 +338,19 @@ impl<T> LocalizedString<T> {
//TODO: this recomputes the string if either the language has changed,
//or *anytime* we have arguments. Ideally we would be using a lens
//to only recompute when our actual data has changed.
if self.args.is_some()
|| self.resolved_lang.as_ref() != Some(&env.localization_manager().current_locale)
{
let manager = match env.localization_manager() {
Some(manager) => manager,
None => return false,
};

if self.args.is_some() || self.resolved_lang.as_ref() != Some(&manager.current_locale) {
let args: Option<FluentArgs> = self
.args
.as_ref()
.map(|a| a.iter().map(|(k, v)| (*k, (v.0)(data, env))).collect());

self.resolved_lang = Some(env.localization_manager().current_locale.clone());
let next = env.localization_manager().localize(self.key, args.as_ref());
self.resolved_lang = Some(manager.current_locale.clone());
let next = manager.localize(self.key, args.as_ref());
let result = next != self.resolved;
self.resolved = next;
result
Expand Down
2 changes: 1 addition & 1 deletion druid/src/scroll_component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ mod tests {
}

fn test_env() -> Env {
Env::default()
Env::empty()
.adding(theme::SCROLLBAR_WIDTH, TEST_SCROLLBAR_WIDTH)
.adding(theme::SCROLLBAR_PAD, TEST_SCROLLBAR_PAD)
.adding(theme::SCROLLBAR_MIN_SIZE, TEST_SCROLLBAR_MIN_SIZE)
Expand Down
2 changes: 1 addition & 1 deletion druid/src/tests/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl<T: Data> Harness<'_, T> {

let inner = Inner {
data,
env: Env::default(),
env: Env::with_default_i10n(),
window,
cmds: Default::default(),
};
Expand Down
5 changes: 0 additions & 5 deletions druid/src/theme.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,3 @@ pub(crate) fn add_to_env(env: Env) -> Env {
.with_size(15.0),
)
}

#[deprecated(since = "0.7.0", note = "use Env::default() instead")]
pub fn init() -> Env {
Env::default()
}

0 comments on commit 5739d33

Please sign in to comment.