D7575: hg-core: vendor Facebook's configparser crate

Alphare (Raphaël Gomès) phabricator at mercurial-scm.org
Mon Dec 9 06:38:43 EST 2019


Alphare added inline comments.

INLINE COMMENTS

> c_api.rs:1
> +/*
> + * Copyright (c) Facebook, Inc. and its affiliates.

Do we need bindings to C ? I don't see what the use-case is at this stage of the Rust development.

> quark wrote in hg.rs:262-263
> You might want to respect `$PAGER`. We ignored it to reduce support burden for mis-configuration.

+1

> hg.rs:226
> +impl ConfigSetHgExt for ConfigSet {
> +    fn load_system(&mut self) -> Vec<Error> {
> +        let opts = Options::new().source("system").process_hgplain();

This function looks very Facebook specific, I don't think we want to include it at all.

> hg.rs:413
> +
> +impl FromConfigValue for String {
> +    fn try_from_bytes(bytes: &[u8]) -> Result<Self> {

Our current strategy is to assume config files to be in local encoding, not UTF-8. This makes sense from Facebook's perspective, but now as an upstream solution (see previous work on `HgPath`).

> hg.rs:595
> +                    let branch = {
> +                        match parts.last() {
> +                            None => 1,

Is this hack still needed as of `1.34.2`?

> spec.pest:63
> +compound = _{ (config_item | section | comment_line | directive | blank_line ) }
> +file = _{ SOI ~ compound ~ (new_line ~ compound)* ~ EOI }

`pest` is really cool.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7575/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7575

To: indygreg, #hg-reviewers
Cc: Alphare, quark, durin42, kevincox, mjpieters, mercurial-devel


More information about the Mercurial-devel mailing list