rhg: Port Python’s `ui.configlist` as `Config::get_list`

This new method is not used yet outside of its own unit tests,
so this changeset should make no observable change.

The Rust parser implementation attempts to exactly replicate the behavior of
the Python one, even in edge cases where that behavior is… surprising.
New unit tests capture some of these edge cases.

This started as a line-by-line port. The main changes are:

* Pass around a parser mode enum instead of parser functions
* Inline the whole parser into one function
* Use `[u8]::get` which returns an `Option`,
  instead of indexing  after explicitly checking the length.

Differential Revision: https://phab.mercurial-scm.org/D11389

tests/test-config-parselist.py

@@ -0,0 +1,52 b''
+"""
+List-valued configuration keys have an ad-hoc microsyntax. From `hg help config`:
+
+> List values are separated by whitespace or comma, except when values are
+> placed in double quotation marks:
+>
+>     allow_read = "John Doe, PhD", brian, betty
+>
+> Quotation marks can be escaped by prefixing them with a backslash. Only
+> quotation marks at the beginning of a word is counted as a quotation
+> (e.g., ``foo"bar baz`` is the list of ``foo"bar`` and ``baz``).
+
+That help documentation is fairly light on details, the actual parser has many
+other edge cases. This test tries to cover them.
+"""
+
+from mercurial.utils import stringutil
+
+
+def assert_parselist(input, expected):
+    result = stringutil.parselist(input)
+    if result != expected:
+        raise AssertionError(
+            "parse_input(%r)\n     got %r\nexpected %r"
+            % (input, result, expected)
+        )
+
+
+# Keep these Python tests in sync with the Rust ones in `rust/hg-core/src/config/values.rs`
+
+assert_parselist(b'', [])
+assert_parselist(b',', [])
+assert_parselist(b'A', [b'A'])
+assert_parselist(b'B,B', [b'B', b'B'])
+assert_parselist(b', C, ,C,', [b'C', b'C'])
+assert_parselist(b'"', [b'"'])
+assert_parselist(b'""', [b'', b''])
+assert_parselist(b'D,"', [b'D', b'"'])
+assert_parselist(b'E,""', [b'E', b'', b''])
+assert_parselist(b'"F,F"', [b'F,F'])
+assert_parselist(b'"G,G', [b'"G', b'G'])
+assert_parselist(b'"H \\",\\"H', [b'"H', b',', b'H'])
+assert_parselist(b'I,I"', [b'I', b'I"'])
+assert_parselist(b'J,"J', [b'J', b'"J'])
+assert_parselist(b'K K', [b'K', b'K'])
+assert_parselist(b'"K" K', [b'K', b'K'])
+assert_parselist(b'L\tL', [b'L', b'L'])
+assert_parselist(b'"L"\tL', [b'L', b'', b'L'])
+assert_parselist(b'M\x0bM', [b'M', b'M'])
+assert_parselist(b'"M"\x0bM', [b'M', b'', b'M'])
+assert_parselist(b'"N"  , ,"', [b'N"'])
+assert_parselist(b'" ,O,  ', [b'"', b'O'])

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

@@ -1,486 +1,496 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.
 
 use super::layer;
 use super::values;
 use crate::config::layer::{
     ConfigError, ConfigLayer, ConfigOrigin, ConfigValue,
 };
 use crate::utils::files::get_bytes_from_os_str;
 use crate::utils::SliceExt;
 use format_bytes::{write_bytes, DisplayBytes};
 use std::collections::HashSet;
 use std::env;
 use std::fmt;
 use std::path::{Path, PathBuf};
 use std::str;
 
 use crate::errors::{HgResultExt, IoResultExt};
 
 /// 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>,
 }
 
 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 ConfigValueParseError {
     pub origin: ConfigOrigin,
     pub line: Option<usize>,
     pub section: Vec<u8>,
     pub item: Vec<u8>,
     pub value: Vec<u8>,
     pub expected_type: &'static str,
 }
 
 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)
         )
     }
 }
 
 impl Config {
     /// 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 { layers: Vec::new() };
         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_config(
         &mut self,
         cli_config_args: impl IntoIterator<Item = impl AsRef<[u8]>>,
     ) -> Result<(), ConfigError> {
         if let Some(layer) = ConfigLayer::parse_cli_args(cli_config_args)? {
             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 })
     }
 
     /// 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,
         };
         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)
     }
 
     fn get_parse<'config, T: 'config>(
         &'config self,
         section: &[u8],
         item: &[u8],
         expected_type: &'static str,
         parse: impl Fn(&'config [u8]) -> Option<T>,
     ) -> Result<Option<T>, ConfigValueParseError> {
         match self.get_inner(&section, &item) {
             Some((layer, v)) => match parse(&v.bytes) {
                 Some(b) => Ok(Some(b)),
                 None => Err(ConfigValueParseError {
                     origin: layer.origin.to_owned(),
                     line: v.line,
                     value: v.bytes.to_owned(),
                     section: section.to_owned(),
                     item: item.to_owned(),
                     expected_type,
                 }),
             },
             None => Ok(None),
         }
     }
 
     /// 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>, ConfigValueParseError> {
         self.get_parse(section, item, "ASCII or UTF-8 string", |value| {
             str::from_utf8(value).ok()
         })
     }
 
     /// 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>, ConfigValueParseError> {
         self.get_parse(section, item, "valid integer", |value| {
             str::from_utf8(value).ok()?.parse().ok()
         })
     }
 
     /// 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>, ConfigValueParseError> {
         self.get_parse(section, item, "byte quantity", values::parse_byte_size)
     }
 
     /// 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>, ConfigValueParseError> {
         self.get_parse(section, item, "boolean", values::parse_bool)
     }
 
     /// 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, ConfigValueParseError> {
         Ok(self.get_option(section, item)?.unwrap_or(false))
     }
 
     /// Returns the corresponding list-value in the config if found, or `None`.
     ///
     /// This is appropriate for new configuration keys. The value syntax is
     /// **not** the same as most existing list-valued config, which has Python
     /// parsing implemented in `parselist()` in
     /// `mercurial/utils/stringutil.py`. Faithfully porting that parsing
     /// algorithm to Rust (including behavior that are arguably bugs)
     /// turned out to be non-trivial and hasn’t been completed as of this
     /// writing.
     ///
     /// Instead, the "simple" syntax is: split on comma, then trim leading and
     /// trailing whitespace of each component. Quotes or backslashes are not
     /// interpreted in any way. Commas are mandatory between values. Values
     /// that contain a comma are not supported.
     pub fn get_simple_list(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Option<impl Iterator<Item = &[u8]>> {
         self.get(section, item).map(|value| {
             value
                 .split(|&byte| byte == b',')
                 .map(|component| component.trim())
         })
     }
 
+    /// 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 layer and the value of the first one found, or `None`.
     fn get_inner(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Option<(&ConfigLayer, &ConfigValue)> {
         for layer in self.layers.iter().rev() {
             if !layer.trusted {
                 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()
     }
 
     /// 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
     }
 }
 
 #[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());
     }
 }

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

@@ -1,61 +1,273 b''
 //! Parsing functions for various type of configuration values.
 //!
 //! Returning `None` indicates a syntax error. Using a `Result` would be more
 //! correct but would take more boilerplate for converting between error types,
 //! compared to using `.ok()` on inner results of various error types to
 //! convert them all to options. The `Config::get_parse` method later converts
 //! those options to results with `ConfigValueParseError`, which contains
 //! details about where the value came from (but omits details of what’s
 //! invalid inside the value).
 
+use crate::utils::SliceExt;
+
 pub(super) fn parse_bool(v: &[u8]) -> Option<bool> {
     match v.to_ascii_lowercase().as_slice() {
         b"1" | b"yes" | b"true" | b"on" | b"always" => Some(true),
         b"0" | b"no" | b"false" | b"off" | b"never" => Some(false),
         _ => None,
     }
 }
 
 pub(super) fn parse_byte_size(value: &[u8]) -> Option<u64> {
     let value = std::str::from_utf8(value).ok()?.to_ascii_lowercase();
     const UNITS: &[(&str, u64)] = &[
         ("g", 1 << 30),
         ("gb", 1 << 30),
         ("m", 1 << 20),
         ("mb", 1 << 20),
         ("k", 1 << 10),
         ("kb", 1 << 10),
         ("b", 1 << 0), // Needs to be last
     ];
     for &(unit, multiplier) in UNITS {
         // TODO: use `value.strip_suffix(unit)` when we require Rust 1.45+
         if value.ends_with(unit) {
             let value_before_unit = &value[..value.len() - unit.len()];
             let float: f64 = value_before_unit.trim().parse().ok()?;
             if float >= 0.0 {
                 return Some((float * multiplier as f64).round() as u64);
             } else {
                 return None;
             }
         }
     }
     value.parse().ok()
 }
 
+/// Parse a config value as a list of sub-values.
+///
+/// Ported from `parselist` in `mercurial/utils/stringutil.py`
+
+// Note: keep behavior in sync with the Python one.
+
+// Note: this could return `Vec<Cow<[u8]>>` instead and borrow `input` when
+// possible (when there’s no backslash-escapes) but this is probably not worth
+// the complexity as config is presumably not accessed inside
+// preformance-sensitive loops.
+pub(super) fn parse_list(input: &[u8]) -> Vec<Vec<u8>> {
+    // Port of Python’s `value.lstrip(b' ,\n')`
+    // TODO: is this really what we want?
+    let input =
+        input.trim_start_matches(|b| b == b' ' || b == b',' || b == b'\n');
+    parse_list_without_trim_start(input)
+}
+
+fn parse_list_without_trim_start(input: &[u8]) -> Vec<Vec<u8>> {
+    // Start of port of Python’s `_configlist`
+    let input = input.trim_end_matches(|b| b == b' ' || b == b',');
+    if input.is_empty() {
+        return Vec::new();
+    }
+
+    // Just to make “a string” less confusable with “a list of strings”.
+    type ByteString = Vec<u8>;
+
+    // These correspond to Python’s…
+    let mut mode = ParserMode::Plain; // `parser`
+    let mut values = Vec::new(); // `parts[:-1]`
+    let mut next_value = ByteString::new(); // `parts[-1]`
+    let mut offset = 0; // `offset`
+
+    // Setting `parser` to `None` is instead handled by returning immediately
+    enum ParserMode {
+        Plain,
+        Quoted,
+    }
+
+    loop {
+        match mode {
+            ParserMode::Plain => {
+                // Start of port of Python’s `_parse_plain`
+                let mut whitespace = false;
+                while let Some(&byte) = input.get(offset) {
+                    if is_space(byte) || byte == b',' {
+                        whitespace = true;
+                        offset += 1;
+                    } else {
+                        break;
+                    }
+                }
+                if let Some(&byte) = input.get(offset) {
+                    if whitespace {
+                        values.push(std::mem::take(&mut next_value))
+                    }
+                    if byte == b'"' && next_value.is_empty() {
+                        mode = ParserMode::Quoted;
+                    } else {
+                        if byte == b'"' && next_value.ends_with(b"\\") {
+                            next_value.pop();
+                        }
+                        next_value.push(byte);
+                    }
+                    offset += 1;
+                } else {
+                    values.push(next_value);
+                    return values;
+                }
+            }
+            ParserMode::Quoted => {
+                // Start of port of Python’s `_parse_quote`
+                if let Some(&byte) = input.get(offset) {
+                    if byte == b'"' {
+                        // The input contains a quoted zero-length value `""`
+                        debug_assert_eq!(next_value, b"");
+                        values.push(std::mem::take(&mut next_value));
+                        offset += 1;
+                        while let Some(&byte) = input.get(offset) {
+                            if is_space(byte) || byte == b',' {
+                                offset += 1;
+                            } else {
+                                break;
+                            }
+                        }
+                        mode = ParserMode::Plain;
+                        continue;
+                    }
+                }
+
+                while let Some(&byte) = input.get(offset) {
+                    if byte == b'"' {
+                        break;
+                    }
+                    if byte == b'\\' && input.get(offset + 1) == Some(&b'"') {
+                        next_value.push(b'"');
+                        offset += 2;
+                    } else {
+                        next_value.push(byte);
+                        offset += 1;
+                    }
+                }
+
+                if offset >= input.len() {
+                    // We didn’t find a closing double-quote,
+                    // so treat the opening one as part of an unquoted value
+                    // instead of delimiting the start of a quoted value.
+
+                    // `next_value` may have had some backslash-escapes
+                    // unescaped. TODO: shouldn’t we use a slice of `input`
+                    // instead?
+                    let mut real_values =
+                        parse_list_without_trim_start(&next_value);
+
+                    if let Some(first) = real_values.first_mut() {
+                        first.insert(0, b'"');
+                        // Drop `next_value`
+                        values.extend(real_values)
+                    } else {
+                        next_value.push(b'"');
+                        values.push(next_value);
+                    }
+                    return values;
+                }
+
+                // We’re not at the end of the input, which means the `while`
+                // loop above ended at at double quote. Skip
+                // over that.
+                offset += 1;
+
+                while let Some(&byte) = input.get(offset) {
+                    if byte == b' ' || byte == b',' {
+                        offset += 1;
+                    } else {
+                        break;
+                    }
+                }
+
+                if offset >= input.len() {
+                    values.push(next_value);
+                    return values;
+                }
+
+                if offset + 1 == input.len() && input[offset] == b'"' {
+                    next_value.push(b'"');
+                    offset += 1;
+                } else {
+                    values.push(std::mem::take(&mut next_value));
+                }
+
+                mode = ParserMode::Plain;
+            }
+        }
+    }
+
+    // https://docs.python.org/3/library/stdtypes.html?#bytes.isspace
+    fn is_space(byte: u8) -> bool {
+        if let b' ' | b'\t' | b'\n' | b'\r' | b'\x0b' | b'\x0c' = byte {
+            true
+        } else {
+            false
+        }
+    }
+}
+
+#[test]
+fn test_parse_list() {
+    // Make `assert_eq` error messages nicer
+    fn as_strings(values: &[Vec<u8>]) -> Vec<String> {
+        values
+            .iter()
+            .map(|v| std::str::from_utf8(v.as_ref()).unwrap().to_owned())
+            .collect()
+    }
+    macro_rules! assert_parse_list {
+        ( $input: expr => [ $( $output: expr ),* ] ) => {
+            assert_eq!(
+                as_strings(&parse_list($input)),
+                as_strings(&[ $( Vec::from(&$output[..]) ),* ]),
+            );
+        }
+    }
+
+    // Keep these Rust tests in sync with the Python ones in
+    // `tests/test-config-parselist.py`
+    assert_parse_list!(b"" => []);
+    assert_parse_list!(b"," => []);
+    assert_parse_list!(b"A" => [b"A"]);
+    assert_parse_list!(b"B,B" => [b"B", b"B"]);
+    assert_parse_list!(b", C, ,C," => [b"C", b"C"]);
+    assert_parse_list!(b"\"" => [b"\""]);
+    assert_parse_list!(b"\"\"" => [b"", b""]);
+    assert_parse_list!(b"D,\"" => [b"D", b"\""]);
+    assert_parse_list!(b"E,\"\"" => [b"E", b"", b""]);
+    assert_parse_list!(b"\"F,F\"" => [b"F,F"]);
+    assert_parse_list!(b"\"G,G" => [b"\"G", b"G"]);
+    assert_parse_list!(b"\"H \\\",\\\"H" => [b"\"H", b",", b"H"]);
+    assert_parse_list!(b"I,I\"" => [b"I", b"I\""]);
+    assert_parse_list!(b"J,\"J" => [b"J", b"\"J"]);
+    assert_parse_list!(b"K K" => [b"K", b"K"]);
+    assert_parse_list!(b"\"K\" K" => [b"K", b"K"]);
+    assert_parse_list!(b"L\tL" => [b"L", b"L"]);
+    assert_parse_list!(b"\"L\"\tL" => [b"L", b"", b"L"]);
+    assert_parse_list!(b"M\x0bM" => [b"M", b"M"]);
+    assert_parse_list!(b"\"M\"\x0bM" => [b"M", b"", b"M"]);
+    assert_parse_list!(b"\"N\"  , ,\"" => [b"N\""]);
+    assert_parse_list!(b"\" ,O,  " => [b"\"", b"O"]);
+}
+
 #[test]
 fn test_parse_byte_size() {
     assert_eq!(parse_byte_size(b""), None);
     assert_eq!(parse_byte_size(b"b"), None);
 
     assert_eq!(parse_byte_size(b"12"), Some(12));
     assert_eq!(parse_byte_size(b"12b"), Some(12));
     assert_eq!(parse_byte_size(b"12 b"), Some(12));
     assert_eq!(parse_byte_size(b"12.1 b"), Some(12));
     assert_eq!(parse_byte_size(b"1.1 K"), Some(1126));
     assert_eq!(parse_byte_size(b"1.1 kB"), Some(1126));
 
     assert_eq!(parse_byte_size(b"-12 b"), None);
     assert_eq!(parse_byte_size(b"-0.1 b"), None);
     assert_eq!(parse_byte_size(b"0.1 b"), Some(0));
     assert_eq!(parse_byte_size(b"12.1 b"), Some(12));
 }