rust-config: fix incorrect coercion of null values to false

As explained in the previous changeset:

Probably being too trigger happy about boolean values, I incorrectly set
the transform for a `None` to a `Some(false)`. It would cause for example
the `ui.formatted` value to be set to `Some(false)`, which turns off the colors
among other things, when `None` would trigger the automatic behavior.

rust/hg-core/src/config/config_items.rs

@@ -1,725 +1,725 b''
 //! Code for parsing default Mercurial config items.
 use itertools::Itertools;
 use serde::Deserialize;
 
 use crate::{errors::HgError, exit_codes, FastHashMap};
 
 /// Corresponds to the structure of `mercurial/configitems.toml`.
 #[derive(Debug, Deserialize)]
 pub struct ConfigItems {
     items: Vec<DefaultConfigItem>,
     templates: FastHashMap<String, Vec<TemplateItem>>,
     #[serde(rename = "template-applications")]
     template_applications: Vec<TemplateApplication>,
 }
 
 /// Corresponds to a config item declaration in `mercurial/configitems.toml`.
 #[derive(Clone, Debug, PartialEq, Deserialize)]
 #[serde(try_from = "RawDefaultConfigItem")]
 pub struct DefaultConfigItem {
     /// Section of the config the item is in (e.g. `[merge-tools]`)
     section: String,
     /// Name of the item (e.g. `meld.gui`)
     name: String,
     /// Default value (can be dynamic, see [`DefaultConfigItemType`])
     default: Option<DefaultConfigItemType>,
     /// If the config option is generic (e.g. `merge-tools.*`), defines
     /// the priority of this item relative to other generic items.
     /// If we're looking for <pattern>, then all generic items within the same
     /// section will be sorted by order of priority, and the first regex match
     /// against `name` is returned.
     #[serde(default)]
     priority: Option<isize>,
     /// Aliases, if any. Each alias is a tuple of `(section, name)` for each
     /// option that is aliased to this one.
     #[serde(default)]
     alias: Vec<(String, String)>,
     /// Whether the config item is marked as experimental
     #[serde(default)]
     experimental: bool,
     /// The (possibly empty) docstring for the item
     #[serde(default)]
     documentation: String,
     /// Whether the item is part of an in-core extension. This allows us to
     /// hide them if the extension is not enabled, to preserve legacy
     /// behavior.
     #[serde(default)]
     in_core_extension: Option<String>,
 }
 
 /// Corresponds to the raw (i.e. on disk) structure of config items. Used as
 /// an intermediate step in deserialization.
 #[derive(Clone, Debug, Deserialize)]
 struct RawDefaultConfigItem {
     section: String,
     name: String,
     default: Option<toml::Value>,
     #[serde(rename = "default-type")]
     default_type: Option<String>,
     #[serde(default)]
     priority: isize,
     #[serde(default)]
     generic: bool,
     #[serde(default)]
     alias: Vec<(String, String)>,
     #[serde(default)]
     experimental: bool,
     #[serde(default)]
     documentation: String,
     #[serde(default)]
     in_core_extension: Option<String>,
 }
 
 impl TryFrom<RawDefaultConfigItem> for DefaultConfigItem {
     type Error = HgError;
 
     fn try_from(value: RawDefaultConfigItem) -> Result<Self, Self::Error> {
         Ok(Self {
             section: value.section,
             name: value.name,
             default: raw_default_to_concrete(
                 value.default_type,
                 value.default,
             )?,
             priority: if value.generic {
                 Some(value.priority)
             } else {
                 None
             },
             alias: value.alias,
             experimental: value.experimental,
             documentation: value.documentation,
             in_core_extension: value.in_core_extension,
         })
     }
 }
 
 impl DefaultConfigItem {
     fn is_generic(&self) -> bool {
         self.priority.is_some()
     }
 
     pub fn in_core_extension(&self) -> Option<&str> {
         self.in_core_extension.as_deref()
     }
 
     pub fn section(&self) -> &str {
         self.section.as_ref()
     }
 }
 
 impl<'a> TryFrom<&'a DefaultConfigItem> for Option<&'a str> {
     type Error = HgError;
 
     fn try_from(
         value: &'a DefaultConfigItem,
     ) -> Result<Option<&'a str>, Self::Error> {
         match &value.default {
             Some(default) => {
                 let err = HgError::abort(
                     format!(
                         "programming error: wrong query on config item '{}.{}'",
                         value.section,
                         value.name
                     ),
                     exit_codes::ABORT,
                     Some(format!(
                         "asked for '&str', type of default is '{}'",
                         default.type_str()
                     )),
                 );
                 match default {
                     DefaultConfigItemType::Primitive(toml::Value::String(
                         s,
                     )) => Ok(Some(s)),
                     _ => Err(err),
                 }
             }
             None => Ok(None),
         }
     }
 }
 
 impl<'a> TryFrom<&'a DefaultConfigItem> for Option<&'a [u8]> {
     type Error = HgError;
 
     fn try_from(
         value: &'a DefaultConfigItem,
     ) -> Result<Option<&'a [u8]>, Self::Error> {
         match &value.default {
             Some(default) => {
                 let err = HgError::abort(
                     format!(
                         "programming error: wrong query on config item '{}.{}'",
                         value.section,
                         value.name
                     ),
                     exit_codes::ABORT,
                     Some(format!(
                         "asked for bytes, type of default is '{}', \
                         which cannot be interpreted as bytes",
                         default.type_str()
                     )),
                 );
                 match default {
                     DefaultConfigItemType::Primitive(p) => {
                         Ok(p.as_str().map(str::as_bytes))
                     }
                     _ => Err(err),
                 }
             }
             None => Ok(None),
         }
     }
 }
 
 impl TryFrom<&DefaultConfigItem> for Option<bool> {
     type Error = HgError;
 
     fn try_from(value: &DefaultConfigItem) -> Result<Self, Self::Error> {
         match &value.default {
             Some(default) => {
                 let err = HgError::abort(
                     format!(
                         "programming error: wrong query on config item '{}.{}'",
                         value.section,
                         value.name
                     ),
                     exit_codes::ABORT,
                     Some(format!(
                         "asked for 'bool', type of default is '{}'",
                         default.type_str()
                     )),
                 );
                 match default {
                     DefaultConfigItemType::Primitive(
                         toml::Value::Boolean(b),
                     ) => Ok(Some(*b)),
                     _ => Err(err),
                 }
             }
-            None => Ok(Some(false)),
+            None => Ok(None),
         }
     }
 }
 
 impl TryFrom<&DefaultConfigItem> for Option<u32> {
     type Error = HgError;
 
     fn try_from(value: &DefaultConfigItem) -> Result<Self, Self::Error> {
         match &value.default {
             Some(default) => {
                 let err = HgError::abort(
                     format!(
                         "programming error: wrong query on config item '{}.{}'",
                         value.section,
                         value.name
                     ),
                     exit_codes::ABORT,
                     Some(format!(
                         "asked for 'u32', type of default is '{}'",
                         default.type_str()
                     )),
                 );
                 match default {
                     DefaultConfigItemType::Primitive(
                         toml::Value::Integer(b),
                     ) => {
                         Ok(Some((*b).try_into().expect("TOML integer to u32")))
                     }
                     _ => Err(err),
                 }
             }
             None => Ok(None),
         }
     }
 }
 
 impl TryFrom<&DefaultConfigItem> for Option<u64> {
     type Error = HgError;
 
     fn try_from(value: &DefaultConfigItem) -> Result<Self, Self::Error> {
         match &value.default {
             Some(default) => {
                 let err = HgError::abort(
                     format!(
                         "programming error: wrong query on config item '{}.{}'",
                         value.section,
                         value.name
                     ),
                     exit_codes::ABORT,
                     Some(format!(
                         "asked for 'u64', type of default is '{}'",
                         default.type_str()
                     )),
                 );
                 match default {
                     DefaultConfigItemType::Primitive(
                         toml::Value::Integer(b),
                     ) => {
                         Ok(Some((*b).try_into().expect("TOML integer to u64")))
                     }
                     _ => Err(err),
                 }
             }
             None => Ok(None),
         }
     }
 }
 
 /// Allows abstracting over more complex default values than just primitives.
 /// The former `configitems.py` contained some dynamic code that is encoded
 /// in this enum.
 #[derive(Debug, PartialEq, Clone, Deserialize)]
 pub enum DefaultConfigItemType {
     /// Some primitive type (string, integer, boolean)
     Primitive(toml::Value),
     /// A dynamic value that will be given by the code at runtime
     Dynamic,
     /// An lazily-returned array (possibly only relevant in the Python impl)
     /// Example: `lambda: [b"zstd", b"zlib"]`
     Lambda(Vec<String>),
     /// For now, a special case for `web.encoding` that points to the
     /// `encoding.encoding` module in the Python impl so that local encoding
     /// is correctly resolved at runtime
     LazyModule(String),
     ListType,
 }
 
 impl DefaultConfigItemType {
     pub fn type_str(&self) -> &str {
         match self {
             DefaultConfigItemType::Primitive(primitive) => {
                 primitive.type_str()
             }
             DefaultConfigItemType::Dynamic => "dynamic",
             DefaultConfigItemType::Lambda(_) => "lambda",
             DefaultConfigItemType::LazyModule(_) => "lazy_module",
             DefaultConfigItemType::ListType => "list_type",
         }
     }
 }
 
 /// Most of the fields are shared with [`DefaultConfigItem`].
 #[derive(Debug, Clone, Deserialize)]
 #[serde(try_from = "RawTemplateItem")]
 struct TemplateItem {
     suffix: String,
     default: Option<DefaultConfigItemType>,
     priority: Option<isize>,
     #[serde(default)]
     alias: Vec<(String, String)>,
     #[serde(default)]
     experimental: bool,
     #[serde(default)]
     documentation: String,
 }
 
 /// Corresponds to the raw (i.e. on disk) representation of a template item.
 /// Used as an intermediate step in deserialization.
 #[derive(Clone, Debug, Deserialize)]
 struct RawTemplateItem {
     suffix: String,
     default: Option<toml::Value>,
     #[serde(rename = "default-type")]
     default_type: Option<String>,
     #[serde(default)]
     priority: isize,
     #[serde(default)]
     generic: bool,
     #[serde(default)]
     alias: Vec<(String, String)>,
     #[serde(default)]
     experimental: bool,
     #[serde(default)]
     documentation: String,
 }
 
 impl TemplateItem {
     fn into_default_item(
         self,
         application: TemplateApplication,
     ) -> DefaultConfigItem {
         DefaultConfigItem {
             section: application.section,
             name: application
                 .prefix
                 .map(|prefix| format!("{}.{}", prefix, self.suffix))
                 .unwrap_or(self.suffix),
             default: self.default,
             priority: self.priority,
             alias: self.alias,
             experimental: self.experimental,
             documentation: self.documentation,
             in_core_extension: None,
         }
     }
 }
 
 impl TryFrom<RawTemplateItem> for TemplateItem {
     type Error = HgError;
 
     fn try_from(value: RawTemplateItem) -> Result<Self, Self::Error> {
         Ok(Self {
             suffix: value.suffix,
             default: raw_default_to_concrete(
                 value.default_type,
                 value.default,
             )?,
             priority: if value.generic {
                 Some(value.priority)
             } else {
                 None
             },
             alias: value.alias,
             experimental: value.experimental,
             documentation: value.documentation,
         })
     }
 }
 
 /// Transforms the on-disk string-based representation of complex default types
 /// to the concrete [`DefaultconfigItemType`].
 fn raw_default_to_concrete(
     default_type: Option<String>,
     default: Option<toml::Value>,
 ) -> Result<Option<DefaultConfigItemType>, HgError> {
     Ok(match default_type.as_deref() {
         None => default.as_ref().map(|default| {
             DefaultConfigItemType::Primitive(default.to_owned())
         }),
         Some("dynamic") => Some(DefaultConfigItemType::Dynamic),
         Some("list_type") => Some(DefaultConfigItemType::ListType),
         Some("lambda") => match &default {
             Some(default) => Some(DefaultConfigItemType::Lambda(
                 default.to_owned().try_into().map_err(|e| {
                     HgError::abort(
                         e.to_string(),
                         exit_codes::ABORT,
                         Some("Check 'mercurial/configitems.toml'".into()),
                     )
                 })?,
             )),
             None => {
                 return Err(HgError::abort(
                     "lambda defined with no return value".to_string(),
                     exit_codes::ABORT,
                     Some("Check 'mercurial/configitems.toml'".into()),
                 ))
             }
         },
         Some("lazy_module") => match &default {
             Some(default) => {
                 Some(DefaultConfigItemType::LazyModule(match default {
                     toml::Value::String(module) => module.to_owned(),
                     _ => {
                         return Err(HgError::abort(
                             "lazy_module module name should be a string"
                                 .to_string(),
                             exit_codes::ABORT,
                             Some("Check 'mercurial/configitems.toml'".into()),
                         ))
                     }
                 }))
             }
             None => {
                 return Err(HgError::abort(
                     "lazy_module should have a default value".to_string(),
                     exit_codes::ABORT,
                     Some("Check 'mercurial/configitems.toml'".into()),
                 ))
             }
         },
         Some(invalid) => {
             return Err(HgError::abort(
                 format!("invalid default_type '{}'", invalid),
                 exit_codes::ABORT,
                 Some("Check 'mercurial/configitems.toml'".into()),
             ))
         }
     })
 }
 
 #[derive(Debug, Clone, Deserialize)]
 struct TemplateApplication {
     template: String,
     section: String,
     #[serde(default)]
     prefix: Option<String>,
 }
 
 /// Represents the (dynamic) set of default core Mercurial config items from
 /// `mercurial/configitems.toml`.
 #[derive(Clone, Debug, Default)]
 pub struct DefaultConfig {
     /// Mapping of section -> (mapping of name -> item)
     items: FastHashMap<String, FastHashMap<String, DefaultConfigItem>>,
 }
 
 impl DefaultConfig {
     pub fn empty() -> DefaultConfig {
         Self {
             items: Default::default(),
         }
     }
 
     /// Returns `Self`, given the contents of `mercurial/configitems.toml`
     #[logging_timer::time("trace")]
     pub fn from_contents(contents: &str) -> Result<Self, HgError> {
         let mut from_file: ConfigItems =
             toml::from_str(contents).map_err(|e| {
                 HgError::abort(
                     e.to_string(),
                     exit_codes::ABORT,
                     Some("Check 'mercurial/configitems.toml'".into()),
                 )
             })?;
 
         let mut flat_items = from_file.items;
 
         for application in from_file.template_applications.drain(..) {
             match from_file.templates.get(&application.template) {
                 None => return Err(
                     HgError::abort(
                         format!(
                             "template application refers to undefined template '{}'",
                             application.template
                         ),
                         exit_codes::ABORT,
                         Some("Check 'mercurial/configitems.toml'".into())
                     )
                 ),
                 Some(template_items) => {
                     for template_item in template_items {
                         flat_items.push(
                             template_item
                                 .clone()
                                 .into_default_item(application.clone()),
                         )
                     }
                 }
             };
         }
 
         let items = flat_items.into_iter().fold(
             FastHashMap::default(),
             |mut acc, item| {
                 acc.entry(item.section.to_owned())
                     .or_insert_with(|| {
                         let mut section = FastHashMap::default();
                         section.insert(item.name.to_owned(), item.to_owned());
                         section
                     })
                     .insert(item.name.to_owned(), item);
                 acc
             },
         );
 
         Ok(Self { items })
     }
 
     /// Return the default config item that matches `section` and `item`.
     pub fn get(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Option<&DefaultConfigItem> {
         // Core items must be valid UTF-8
         let section = String::from_utf8_lossy(section);
         let section_map = self.items.get(section.as_ref())?;
         let item_name_lossy = String::from_utf8_lossy(item);
         match section_map.get(item_name_lossy.as_ref()) {
             Some(item) => Some(item),
             None => {
                 for generic_item in section_map
                     .values()
                     .filter(|item| item.is_generic())
                     .sorted_by_key(|item| match item.priority {
                         Some(priority) => (priority, &item.name),
                         _ => unreachable!(),
                     })
                 {
                     if regex::bytes::Regex::new(&generic_item.name)
                         .expect("invalid regex in configitems")
                         .is_match(item)
                     {
                         return Some(generic_item);
                     }
                 }
                 None
             }
         }
     }
 }
 
 #[cfg(test)]
 mod tests {
     use crate::config::config_items::{
         DefaultConfigItem, DefaultConfigItemType,
     };
 
     use super::DefaultConfig;
 
     #[test]
     fn test_config_read() {
         let contents = r#"
 [[items]]
 section = "alias"
 name = "abcd.*"
 default = 3
 generic = true
 priority = -1
 
 [[items]]
 section = "alias"
 name = ".*"
 default-type = "dynamic"
 generic = true
 
 [[items]]
 section = "cmdserver"
 name = "track-log"
 default-type = "lambda"
 default = [ "chgserver", "cmdserver", "repocache",]
 
 [[items]]
 section = "chgserver"
 name = "idletimeout"
 default = 3600
 
 [[items]]
 section = "cmdserver"
 name = "message-encodings"
 default-type = "list_type"
 
 [[items]]
 section = "web"
 name = "encoding"
 default-type = "lazy_module"
 default = "encoding.encoding"
 
 [[items]]
 section = "command-templates"
 name = "graphnode"
 alias = [["ui", "graphnodetemplate"]]
 documentation = """This is a docstring.
 This is another line \
 but this is not."""
 
 [[items]]
 section = "censor"
 name = "policy"
 default = "abort"
 experimental = true
 
 [[template-applications]]
 template = "diff-options"
 section = "commands"
 prefix = "revert.interactive"
 
 [[template-applications]]
 template = "diff-options"
 section = "diff"
 
 [templates]
 [[templates.diff-options]]
 suffix = "nodates"
 default = false
 
 [[templates.diff-options]]
 suffix = "showfunc"
 default = false
 
 [[templates.diff-options]]
 suffix = "unified"
 "#;
         let res = DefaultConfig::from_contents(contents);
         let config = match res {
             Ok(config) => config,
             Err(e) => panic!("{}", e),
         };
         let expected = DefaultConfigItem {
             section: "censor".into(),
             name: "policy".into(),
             default: Some(DefaultConfigItemType::Primitive("abort".into())),
             priority: None,
             alias: vec![],
             experimental: true,
             documentation: "".into(),
             in_core_extension: None,
         };
         assert_eq!(config.get(b"censor", b"policy"), Some(&expected));
 
         // Test generic priority. The `.*` pattern is wider than `abcd.*`, but
         // `abcd.*` has priority, so it should match first.
         let expected = DefaultConfigItem {
             section: "alias".into(),
             name: "abcd.*".into(),
             default: Some(DefaultConfigItemType::Primitive(3.into())),
             priority: Some(-1),
             alias: vec![],
             experimental: false,
             documentation: "".into(),
             in_core_extension: None,
         };
         assert_eq!(config.get(b"alias", b"abcdsomething"), Some(&expected));
 
         //... but if it doesn't, we should fallback to `.*`
         let expected = DefaultConfigItem {
             section: "alias".into(),
             name: ".*".into(),
             default: Some(DefaultConfigItemType::Dynamic),
             priority: Some(0),
             alias: vec![],
             experimental: false,
             documentation: "".into(),
             in_core_extension: None,
         };
         assert_eq!(config.get(b"alias", b"something"), Some(&expected));
 
         let expected = DefaultConfigItem {
             section: "chgserver".into(),
             name: "idletimeout".into(),
             default: Some(DefaultConfigItemType::Primitive(3600.into())),
             priority: None,
             alias: vec![],
             experimental: false,
             documentation: "".into(),
             in_core_extension: None,
         };
         assert_eq!(config.get(b"chgserver", b"idletimeout"), Some(&expected));
 
         let expected = DefaultConfigItem {
             section: "cmdserver".into(),
             name: "track-log".into(),
             default: Some(DefaultConfigItemType::Lambda(vec![
                 "chgserver".into(),
                 "cmdserver".into(),
                 "repocache".into(),
             ])),
             priority: None,
             alias: vec![],
             experimental: false,
             documentation: "".into(),
             in_core_extension: None,
         };
         assert_eq!(config.get(b"cmdserver", b"track-log"), Some(&expected));
 
         let expected = DefaultConfigItem {
             section: "command-templates".into(),
             name: "graphnode".into(),
             default: None,
             priority: None,
             alias: vec![("ui".into(), "graphnodetemplate".into())],
             experimental: false,
             documentation:
                 "This is a docstring.\nThis is another line but this is not."
                     .into(),
             in_core_extension: None,
         };
         assert_eq!(
             config.get(b"command-templates", b"graphnode"),
             Some(&expected)
         );
     }
 }

rust/hg-core/src/config/mod.rs

@@ -1,811 +1,810 b''
 // config.rs
 //
 // Copyright 2020
 //      Valentin Gatien-Baron,
 //      Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Mercurial config parsing and interfaces.
 
 pub mod config_items;
 mod layer;
 mod plain_info;
 mod values;
 pub use layer::{ConfigError, ConfigOrigin, ConfigParseError};
 use lazy_static::lazy_static;
 pub use plain_info::PlainInfo;
 
 use self::config_items::DefaultConfig;
 use self::config_items::DefaultConfigItem;
 use self::layer::ConfigLayer;
 use self::layer::ConfigValue;
 use crate::errors::HgError;
 use crate::errors::{HgResultExt, IoResultExt};
 use crate::utils::files::get_bytes_from_os_str;
 use format_bytes::{write_bytes, DisplayBytes};
 use std::collections::HashSet;
 use std::env;
 use std::fmt;
 use std::path::{Path, PathBuf};
 use std::str;
 
 lazy_static! {
     static ref DEFAULT_CONFIG: Result<DefaultConfig, HgError> = {
         DefaultConfig::from_contents(include_str!(
             "../../../../mercurial/configitems.toml"
         ))
     };
 }
 
 /// Holds the config values for the current repository
 /// TODO update this docstring once we support more sources
 #[derive(Clone)]
 pub struct Config {
     layers: Vec<layer::ConfigLayer>,
     plain: PlainInfo,
 }
 
 impl DisplayBytes for Config {
     fn display_bytes(
         &self,
         out: &mut dyn std::io::Write,
     ) -> std::io::Result<()> {
         for (index, layer) in self.layers.iter().rev().enumerate() {
             write_bytes!(
                 out,
                 b"==== Layer {} (trusted: {}) ====\n{}",
                 index,
                 if layer.trusted {
                     &b"yes"[..]
                 } else {
                     &b"no"[..]
                 },
                 layer
             )?;
         }
         Ok(())
     }
 }
 
 pub enum ConfigSource {
     /// Absolute path to a config file
     AbsPath(PathBuf),
     /// Already parsed (from the CLI, env, Python resources, etc.)
     Parsed(layer::ConfigLayer),
 }
 
 #[derive(Debug)]
 pub struct ConfigValueParseErrorDetails {
     pub origin: ConfigOrigin,
     pub line: Option<usize>,
     pub section: Vec<u8>,
     pub item: Vec<u8>,
     pub value: Vec<u8>,
     pub expected_type: &'static str,
 }
 
 // boxed to avoid very large Result types
 pub type ConfigValueParseError = Box<ConfigValueParseErrorDetails>;
 
 impl fmt::Display for ConfigValueParseError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         // TODO: add origin and line number information, here and in
         // corresponding python code
         write!(
             f,
             "config error: {}.{} is not a {} ('{}')",
             String::from_utf8_lossy(&self.section),
             String::from_utf8_lossy(&self.item),
             self.expected_type,
             String::from_utf8_lossy(&self.value)
         )
     }
 }
 
 /// Returns true if the config item is disabled by PLAIN or PLAINEXCEPT
 fn should_ignore(plain: &PlainInfo, section: &[u8], item: &[u8]) -> bool {
     // duplication with [_applyconfig] in [ui.py],
     if !plain.is_plain() {
         return false;
     }
     if section == b"alias" {
         return plain.plainalias();
     }
     if section == b"revsetalias" {
         return plain.plainrevsetalias();
     }
     if section == b"templatealias" {
         return plain.plaintemplatealias();
     }
     if section == b"ui" {
         let to_delete: &[&[u8]] = &[
             b"debug",
             b"fallbackencoding",
             b"quiet",
             b"slash",
             b"logtemplate",
             b"message-output",
             b"statuscopies",
             b"style",
             b"traceback",
             b"verbose",
         ];
         return to_delete.contains(&item);
     }
     let sections_to_delete: &[&[u8]] =
         &[b"defaults", b"commands", b"command-templates"];
     sections_to_delete.contains(&section)
 }
 
 impl Config {
     /// The configuration to use when printing configuration-loading errors
     pub fn empty() -> Self {
         Self {
             layers: Vec::new(),
             plain: PlainInfo::empty(),
         }
     }
 
     /// Load system and user configuration from various files.
     ///
     /// This is also affected by some environment variables.
     pub fn load_non_repo() -> Result<Self, ConfigError> {
         let mut config = Self::empty();
         let opt_rc_path = env::var_os("HGRCPATH");
         // HGRCPATH replaces system config
         if opt_rc_path.is_none() {
             config.add_system_config()?
         }
 
         config.add_for_environment_variable("EDITOR", b"ui", b"editor");
         config.add_for_environment_variable("VISUAL", b"ui", b"editor");
         config.add_for_environment_variable("PAGER", b"pager", b"pager");
 
         // These are set by `run-tests.py --rhg` to enable fallback for the
         // entire test suite. Alternatives would be setting configuration
         // through `$HGRCPATH` but some tests override that, or changing the
         // `hg` shell alias to include `--config` but that disrupts tests that
         // print command lines and check expected output.
         config.add_for_environment_variable(
             "RHG_ON_UNSUPPORTED",
             b"rhg",
             b"on-unsupported",
         );
         config.add_for_environment_variable(
             "RHG_FALLBACK_EXECUTABLE",
             b"rhg",
             b"fallback-executable",
         );
 
         // HGRCPATH replaces user config
         if opt_rc_path.is_none() {
             config.add_user_config()?
         }
         if let Some(rc_path) = &opt_rc_path {
             for path in env::split_paths(rc_path) {
                 if !path.as_os_str().is_empty() {
                     if path.is_dir() {
                         config.add_trusted_dir(&path)?
                     } else {
                         config.add_trusted_file(&path)?
                     }
                 }
             }
         }
         Ok(config)
     }
 
     pub fn load_cli_args(
         &mut self,
         cli_config_args: impl IntoIterator<Item = impl AsRef<[u8]>>,
         color_arg: Option<Vec<u8>>,
     ) -> Result<(), ConfigError> {
         if let Some(layer) = ConfigLayer::parse_cli_args(cli_config_args)? {
             self.layers.push(layer)
         }
         if let Some(arg) = color_arg {
             let mut layer = ConfigLayer::new(ConfigOrigin::CommandLineColor);
             layer.add(b"ui"[..].into(), b"color"[..].into(), arg, None);
             self.layers.push(layer)
         }
         Ok(())
     }
 
     fn add_trusted_dir(&mut self, path: &Path) -> Result<(), ConfigError> {
         if let Some(entries) = std::fs::read_dir(path)
             .when_reading_file(path)
             .io_not_found_as_none()?
         {
             let mut file_paths = entries
                 .map(|result| {
                     result.when_reading_file(path).map(|entry| entry.path())
                 })
                 .collect::<Result<Vec<_>, _>>()?;
             file_paths.sort();
             for file_path in &file_paths {
                 if file_path.extension() == Some(std::ffi::OsStr::new("rc")) {
                     self.add_trusted_file(file_path)?
                 }
             }
         }
         Ok(())
     }
 
     fn add_trusted_file(&mut self, path: &Path) -> Result<(), ConfigError> {
         if let Some(data) = std::fs::read(path)
             .when_reading_file(path)
             .io_not_found_as_none()?
         {
             self.layers.extend(ConfigLayer::parse(path, &data)?)
         }
         Ok(())
     }
 
     fn add_for_environment_variable(
         &mut self,
         var: &str,
         section: &[u8],
         key: &[u8],
     ) {
         if let Some(value) = env::var_os(var) {
             let origin = layer::ConfigOrigin::Environment(var.into());
             let mut layer = ConfigLayer::new(origin);
             layer.add(
                 section.to_owned(),
                 key.to_owned(),
                 get_bytes_from_os_str(value),
                 None,
             );
             self.layers.push(layer)
         }
     }
 
     #[cfg(unix)] // TODO: other platforms
     fn add_system_config(&mut self) -> Result<(), ConfigError> {
         let mut add_for_prefix = |prefix: &Path| -> Result<(), ConfigError> {
             let etc = prefix.join("etc").join("mercurial");
             self.add_trusted_file(&etc.join("hgrc"))?;
             self.add_trusted_dir(&etc.join("hgrc.d"))
         };
         let root = Path::new("/");
         // TODO: use `std::env::args_os().next().unwrap()` a.k.a. argv[0]
         // instead? TODO: can this be a relative path?
         let hg = crate::utils::current_exe()?;
         // TODO: this order (per-installation then per-system) matches
         // `systemrcpath()` in `mercurial/scmposix.py`, but
         // `mercurial/helptext/config.txt` suggests it should be reversed
         if let Some(installation_prefix) = hg.parent().and_then(Path::parent) {
             if installation_prefix != root {
                 add_for_prefix(installation_prefix)?
             }
         }
         add_for_prefix(root)?;
         Ok(())
     }
 
     #[cfg(unix)] // TODO: other plateforms
     fn add_user_config(&mut self) -> Result<(), ConfigError> {
         let opt_home = home::home_dir();
         if let Some(home) = &opt_home {
             self.add_trusted_file(&home.join(".hgrc"))?
         }
         let darwin = cfg!(any(target_os = "macos", target_os = "ios"));
         if !darwin {
             if let Some(config_home) = env::var_os("XDG_CONFIG_HOME")
                 .map(PathBuf::from)
                 .or_else(|| opt_home.map(|home| home.join(".config")))
             {
                 self.add_trusted_file(&config_home.join("hg").join("hgrc"))?
             }
         }
         Ok(())
     }
 
     /// Loads in order, which means that the precedence is the same
     /// as the order of `sources`.
     pub fn load_from_explicit_sources(
         sources: Vec<ConfigSource>,
     ) -> Result<Self, ConfigError> {
         let mut layers = vec![];
 
         for source in sources.into_iter() {
             match source {
                 ConfigSource::Parsed(c) => layers.push(c),
                 ConfigSource::AbsPath(c) => {
                     // TODO check if it should be trusted
                     // mercurial/ui.py:427
                     let data = match std::fs::read(&c) {
                         Err(_) => continue, // same as the python code
                         Ok(data) => data,
                     };
                     layers.extend(ConfigLayer::parse(&c, &data)?)
                 }
             }
         }
 
         Ok(Config {
             layers,
             plain: PlainInfo::empty(),
         })
     }
 
     /// Loads the per-repository config into a new `Config` which is combined
     /// with `self`.
     pub(crate) fn combine_with_repo(
         &self,
         repo_config_files: &[PathBuf],
     ) -> Result<Self, ConfigError> {
         let (cli_layers, other_layers) = self
             .layers
             .iter()
             .cloned()
             .partition(ConfigLayer::is_from_command_line);
 
         let mut repo_config = Self {
             layers: other_layers,
             plain: PlainInfo::empty(),
         };
         for path in repo_config_files {
             // TODO: check if this file should be trusted:
             // `mercurial/ui.py:427`
             repo_config.add_trusted_file(path)?;
         }
         repo_config.layers.extend(cli_layers);
         Ok(repo_config)
     }
 
     pub fn apply_plain(&mut self, plain: PlainInfo) {
         self.plain = plain;
     }
 
     /// Returns the default value for the given config item, if any.
     pub fn get_default(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<Option<&DefaultConfigItem>, HgError> {
         let default_config = DEFAULT_CONFIG.as_ref().map_err(|e| {
             HgError::abort(
                 e.to_string(),
                 crate::exit_codes::ABORT,
                 Some("`mercurial/configitems.toml` is not valid".into()),
             )
         })?;
         let default_opt = default_config.get(section, item);
         Ok(default_opt.filter(|default| {
             default
                 .in_core_extension()
                 .map(|extension| {
                     // Only return the default for an in-core extension item
                     // if said extension is enabled
                     self.is_extension_enabled(extension.as_bytes())
                 })
                 .unwrap_or(true)
         }))
     }
 
     /// Return the config item that corresponds to a section + item, a function
     /// to parse from the raw bytes to the expected type (which is passed as
     /// a string only to make debugging easier).
     /// Used by higher-level methods like `get_bool`.
     ///
     /// `fallback_to_default` controls whether the default value (if any) is
     /// returned if nothing is found.
     fn get_parse<'config, T: 'config>(
         &'config self,
         section: &[u8],
         item: &[u8],
         expected_type: &'static str,
         parse: impl Fn(&'config [u8]) -> Option<T>,
         fallback_to_default: bool,
     ) -> Result<Option<T>, HgError>
     where
         Option<T>: TryFrom<&'config DefaultConfigItem, Error = HgError>,
     {
         match self.get_inner(section, item) {
             Some((layer, v)) => match parse(&v.bytes) {
                 Some(b) => Ok(Some(b)),
                 None => Err(Box::new(ConfigValueParseErrorDetails {
                     origin: layer.origin.to_owned(),
                     line: v.line,
                     value: v.bytes.to_owned(),
                     section: section.to_owned(),
                     item: item.to_owned(),
                     expected_type,
                 })
                 .into()),
             },
             None => {
                 if !fallback_to_default {
                     return Ok(None);
                 }
                 match self.get_default(section, item)? {
                     Some(default) => {
                         // Defaults are TOML values, so they're not in the same
                         // shape as in the config files.
                         // First try to convert directly to the expected type
                         let as_t = default.try_into();
                         match as_t {
                             Ok(t) => Ok(t),
                             Err(e) => {
                                 // If it fails, it means that...
                                 let as_bytes: Result<Option<&[u8]>, _> =
                                     default.try_into();
                                 match as_bytes {
                                     Ok(bytes_opt) => {
                                         if let Some(bytes) = bytes_opt {
                                             // ...we should be able to parse it
                                             return Ok(parse(bytes));
                                         }
                                         Err(e)
                                     }
                                     Err(_) => Err(e),
                                 }
                             }
                         }
                     }
                     None => {
                         self.print_devel_warning(section, item)?;
                         Ok(None)
                     }
                 }
             }
         }
     }
 
     fn print_devel_warning(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<(), HgError> {
         let warn_all = self.get_bool(b"devel", b"all-warnings")?;
         let warn_specific = self.get_bool(b"devel", b"warn-config-unknown")?;
         if !warn_all || !warn_specific {
             // We technically shouldn't print anything here since it's not
             // the concern of `hg-core`.
             //
             // We're printing directly to stderr since development warnings
             // are not on by default and surfacing this to consumer crates
             // (like `rhg`) would be more difficult, probably requiring
             // something à la `log` crate.
             //
             // TODO maybe figure out a way of exposing a "warnings" channel
             // that consumer crates can hook into. It would be useful for
             // all other warnings that `hg-core` could expose.
             eprintln!(
                 "devel-warn: accessing unregistered config item: '{}.{}'",
                 String::from_utf8_lossy(section),
                 String::from_utf8_lossy(item),
             );
         }
         Ok(())
     }
 
     /// Returns an `Err` if the first value found is not a valid UTF-8 string.
     /// Otherwise, returns an `Ok(value)` if found, or `None`.
     pub fn get_str(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<Option<&str>, HgError> {
         self.get_parse(
             section,
             item,
             "ASCII or UTF-8 string",
             |value| str::from_utf8(value).ok(),
             true,
         )
     }
 
     /// Same as `get_str`, but doesn't fall back to the default `configitem`
     /// if not defined in the user config.
     pub fn get_str_no_default(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<Option<&str>, HgError> {
         self.get_parse(
             section,
             item,
             "ASCII or UTF-8 string",
             |value| str::from_utf8(value).ok(),
             false,
         )
     }
 
     /// Returns an `Err` if the first value found is not a valid unsigned
     /// integer. Otherwise, returns an `Ok(value)` if found, or `None`.
     pub fn get_u32(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<Option<u32>, HgError> {
         self.get_parse(
             section,
             item,
             "valid integer",
             |value| str::from_utf8(value).ok()?.parse().ok(),
             true,
         )
     }
 
     /// Returns an `Err` if the first value found is not a valid file size
     /// value such as `30` (default unit is bytes), `7 MB`, or `42.5 kb`.
     /// Otherwise, returns an `Ok(value_in_bytes)` if found, or `None`.
     pub fn get_byte_size(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<Option<u64>, HgError> {
         self.get_parse(
             section,
             item,
             "byte quantity",
             values::parse_byte_size,
             true,
         )
     }
 
     /// Returns an `Err` if the first value found is not a valid boolean.
     /// Otherwise, returns an `Ok(option)`, where `option` is the boolean if
     /// found, or `None`.
     pub fn get_option(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<Option<bool>, HgError> {
         self.get_parse(section, item, "boolean", values::parse_bool, true)
     }
 
     /// Same as `get_option`, but doesn't fall back to the default `configitem`
     /// if not defined in the user config.
     pub fn get_option_no_default(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<Option<bool>, HgError> {
         self.get_parse(section, item, "boolean", values::parse_bool, false)
     }
 
     /// Returns the corresponding boolean in the config. Returns `Ok(false)`
     /// if the value is not found, an `Err` if it's not a valid boolean.
     pub fn get_bool(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<bool, HgError> {
         Ok(self.get_option(section, item)?.unwrap_or(false))
     }
 
     /// Same as `get_bool`, but doesn't fall back to the default `configitem`
     /// if not defined in the user config.
     pub fn get_bool_no_default(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<bool, HgError> {
         Ok(self.get_option_no_default(section, item)?.unwrap_or(false))
     }
 
     /// Returns `true` if the extension is enabled, `false` otherwise
     pub fn is_extension_enabled(&self, extension: &[u8]) -> bool {
         let value = self.get(b"extensions", extension);
         match value {
             Some(c) => !c.starts_with(b"!"),
             None => false,
         }
     }
 
     /// If there is an `item` value in `section`, parse and return a list of
     /// byte strings.
     pub fn get_list(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Option<Vec<Vec<u8>>> {
         self.get(section, item).map(values::parse_list)
     }
 
     /// Returns the raw value bytes of the first one found, or `None`.
     pub fn get(&self, section: &[u8], item: &[u8]) -> Option<&[u8]> {
         self.get_inner(section, item)
             .map(|(_, value)| value.bytes.as_ref())
     }
 
     /// Returns the raw value bytes of the first one found, or `None`.
     pub fn get_with_origin(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Option<(&[u8], &ConfigOrigin)> {
         self.get_inner(section, item)
             .map(|(layer, value)| (value.bytes.as_ref(), &layer.origin))
     }
 
     /// Returns the layer and the value of the first one found, or `None`.
     fn get_inner(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Option<(&ConfigLayer, &ConfigValue)> {
         // Filter out the config items that are hidden by [PLAIN].
         // This differs from python hg where we delete them from the config.
         let should_ignore = should_ignore(&self.plain, section, item);
         for layer in self.layers.iter().rev() {
             if !layer.trusted {
                 continue;
             }
             //The [PLAIN] config should not affect the defaults.
             //
             // However, PLAIN should also affect the "tweaked" defaults (unless
             // "tweakdefault" is part of "HGPLAINEXCEPT").
             //
             // In practice the tweak-default layer is only added when it is
             // relevant, so we can safely always take it into
             // account here.
             if should_ignore && !(layer.origin == ConfigOrigin::Tweakdefaults)
             {
                 continue;
             }
             if let Some(v) = layer.get(section, item) {
                 return Some((layer, v));
             }
         }
         None
     }
 
     /// Return all keys defined for the given section
     pub fn get_section_keys(&self, section: &[u8]) -> HashSet<&[u8]> {
         self.layers
             .iter()
             .flat_map(|layer| layer.iter_keys(section))
             .collect()
     }
 
     /// Returns whether any key is defined in the given section
     pub fn has_non_empty_section(&self, section: &[u8]) -> bool {
         self.layers
             .iter()
             .any(|layer| layer.has_non_empty_section(section))
     }
 
     /// Yields (key, value) pairs for everything in the given section
     pub fn iter_section<'a>(
         &'a self,
         section: &'a [u8],
     ) -> impl Iterator<Item = (&[u8], &[u8])> + 'a {
         // Deduplicate keys redefined in multiple layers
         let mut keys_already_seen = HashSet::new();
         let mut key_is_new =
             move |&(key, _value): &(&'a [u8], &'a [u8])| -> bool {
                 keys_already_seen.insert(key)
             };
         // This is similar to `flat_map` + `filter_map`, except with a single
         // closure that owns `key_is_new` (and therefore the
         // `keys_already_seen` set):
         let mut layer_iters = self
             .layers
             .iter()
             .rev()
             .map(move |layer| layer.iter_section(section))
             .peekable();
         std::iter::from_fn(move || loop {
             if let Some(pair) = layer_iters.peek_mut()?.find(&mut key_is_new) {
                 return Some(pair);
             } else {
                 layer_iters.next();
             }
         })
     }
 
     /// Get raw values bytes from all layers (even untrusted ones) in order
     /// of precedence.
     #[cfg(test)]
     fn get_all(&self, section: &[u8], item: &[u8]) -> Vec<&[u8]> {
         let mut res = vec![];
         for layer in self.layers.iter().rev() {
             if let Some(v) = layer.get(section, item) {
                 res.push(v.bytes.as_ref());
             }
         }
         res
     }
 
     // a config layer that's introduced by ui.tweakdefaults
     fn tweakdefaults_layer() -> ConfigLayer {
         let mut layer = ConfigLayer::new(ConfigOrigin::Tweakdefaults);
 
         let mut add = |section: &[u8], item: &[u8], value: &[u8]| {
             layer.add(
                 section[..].into(),
                 item[..].into(),
                 value[..].into(),
                 None,
             );
         };
         // duplication of [tweakrc] from [ui.py]
         add(b"ui", b"rollback", b"False");
         add(b"ui", b"statuscopies", b"yes");
         add(b"ui", b"interface", b"curses");
         add(b"ui", b"relative-paths", b"yes");
         add(b"commands", b"grep.all-files", b"True");
         add(b"commands", b"update.check", b"noconflict");
         add(b"commands", b"status.verbose", b"True");
         add(b"commands", b"resolve.explicit-re-merge", b"True");
         add(b"git", b"git", b"1");
         add(b"git", b"showfunc", b"1");
         add(b"git", b"word-diff", b"1");
         layer
     }
 
     // introduce the tweaked defaults as implied by ui.tweakdefaults
     pub fn tweakdefaults(&mut self) {
         self.layers.insert(0, Config::tweakdefaults_layer());
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use pretty_assertions::assert_eq;
     use std::fs::File;
     use std::io::Write;
 
     #[test]
     fn test_include_layer_ordering() {
         let tmpdir = tempfile::tempdir().unwrap();
         let tmpdir_path = tmpdir.path();
         let mut included_file =
             File::create(&tmpdir_path.join("included.rc")).unwrap();
 
         included_file.write_all(b"[section]\nitem=value1").unwrap();
         let base_config_path = tmpdir_path.join("base.rc");
         let mut config_file = File::create(&base_config_path).unwrap();
         let data =
             b"[section]\nitem=value0\n%include included.rc\nitem=value2\n\
               [section2]\ncount = 4\nsize = 1.5 KB\nnot-count = 1.5\nnot-size = 1 ub";
         config_file.write_all(data).unwrap();
 
         let sources = vec![ConfigSource::AbsPath(base_config_path)];
         let config = Config::load_from_explicit_sources(sources)
             .expect("expected valid config");
 
         let (_, value) = config.get_inner(b"section", b"item").unwrap();
         assert_eq!(
             value,
             &ConfigValue {
                 bytes: b"value2".to_vec(),
                 line: Some(4)
             }
         );
 
         let value = config.get(b"section", b"item").unwrap();
         assert_eq!(value, b"value2",);
         assert_eq!(
             config.get_all(b"section", b"item"),
             [b"value2", b"value1", b"value0"]
         );
 
         assert_eq!(config.get_u32(b"section2", b"count").unwrap(), Some(4));
         assert_eq!(
             config.get_byte_size(b"section2", b"size").unwrap(),
             Some(1024 + 512)
         );
         assert!(config.get_u32(b"section2", b"not-count").is_err());
         assert!(config.get_byte_size(b"section2", b"not-size").is_err());
     }
 
     #[test]
     fn test_default_parse() {
         let config = Config::load_from_explicit_sources(vec![])
             .expect("expected valid config");
         let ret = config.get_byte_size(b"cmdserver", b"max-log-size");
         assert!(ret.is_ok(), "{:?}", ret);
 
         let ret = config.get_byte_size(b"ui", b"formatted");
-        // FIXME should be `is_none()`
-        assert!(ret.unwrap().is_some());
+        assert!(ret.unwrap().is_none());
     }
 }