[WIP] Next version rewrite

[milesj]

Rewriting the macro and traits from the ground up...

[JeanMertz]

@milesj I'm curious how far along this rewrite is. Are you mostly going through the motions of fixing/updating tests, or are there fundamental pieces still missing?

I'd be potentially interested in helping pushing this along, depending of course on how much is still missing and requires your ideas/knowledge to complete.

[milesj]

The entire macros crate is being rewritten. I'm not even close to finishing it, maybe only like 30%. I'm hoping to make it easier to use, and easier to extend, to support new features. The current implementation is just not good.

The core/types crates won't be changing much, if at all.

[milesj]

@JeanMertz Seeing as you're here, I have a question for you. Do you have any suggestion on how I could clean up / compact the generated code for merging 2 enums? For example, these configs:

#[derive(Config)]
enum Example {
    #[setting(merge = a)]
    A(bool),
    #[setting(merge = b)]
    B(usize),
    #[setting(merge = c)]
    C(String, i16),
    #[setting(merge = d)]
    D(Option<String>, Vec<String>),
    #[setting(merge = e)]
    E(Option<HashMap<u8, String>>),
}

#[derive(Config)]
enum Example {
    #[setting(nested, merge = append_vec)]
    A(Vec<NestedConfig>),
    #[setting(nested = CustomConfig, merge = merge_hashmap)]
    B(HashMap<String, CustomConfig>),
    #[setting(nested, merge = merge_btreeset)]
    C(Option<BTreeSet<NestedConfig>>),
}

Produce this output... which just feels very messy (and probably not 100% correct).

fn merge(
    &mut self,
    context: &Self::Context,
    mut next: Self,
) -> std::result::Result<(), schematic::ConfigError> {
    match self {
        Self::A(pa) => {
            if let Self::A(na) = next {
                *self = Self::A(a(pa.to_owned(), na, context)?.unwrap_or_default());
            } else {
                *self = next;
            }
        }
        Self::B(pa) => {
            if let Self::B(na) = next {
                *self = Self::B(b(pa.to_owned(), na, context)?.unwrap_or_default());
            } else {
                *self = next;
            }
        }
        Self::C(pa, pb) => {
            if let Self::C(na, nb) = next {
                if let Some((pa, pb)) = c(
                    (pa.to_owned(), pb.to_owned()),
                    (na, nb),
                    context,
                )? {
                    *self = Self::C(pa, pb);
                } else {
                    *self = Self::C(Default::default(), Default::default());
                }
            } else {
                *self = next;
            }
        }
        Self::D(pa, pb) => {
            if let Self::D(na, nb) = next {
                if let Some((pa, pb)) = d(
                    (pa.to_owned(), pb.to_owned()),
                    (na, nb),
                    context,
                )? {
                    *self = Self::D(pa, pb);
                } else {
                    *self = Self::D(Default::default(), Default::default());
                }
            } else {
                *self = next;
            }
        }
        Self::E(pa) => {
            if let Self::E(na) = next {
                *self = Self::E(e(pa.to_owned(), na, context)?.unwrap_or_default());
            } else {
                *self = next;
            }
        }
        _ => {
            *self = next;
        }
    };
    Ok(())
}

fn merge(
    &mut self,
    context: &Self::Context,
    mut next: Self,
) -> std::result::Result<(), schematic::ConfigError> {
    match self {
        Self::A(pa) => {
            if let Self::A(na) = next {
                *self = Self::A(
                    append_vec(pa.to_owned(), na, context)?.unwrap_or_default(),
                );
            } else {
                *self = next;
            }
        }
        Self::B(pa) => {
            if let Self::B(na) = next {
                *self = Self::B(
                    merge_hashmap(pa.to_owned(), na, context)?.unwrap_or_default(),
                );
            } else {
                *self = next;
            }
        }
        Self::C(pa) => {
            if let Self::C(na) = next {
                *self = Self::C(
                    merge_btreeset(pa.to_owned(), na, context)?.unwrap_or_default(),
                );
            } else {
                *self = next;
            }
        }
        _ => {
            *self = next;
        }
    };
    Ok(())
}

Can maybe be handled with a decl macro?

[JeanMertz]

I tried to rewrite your example using a declarative macro, but the macro ended up more complex than the original implementation, so I doubt that's going to net any readability wins, unless I missed some opportunities to simplify it further.

[JeanMertz]

I did notice you can remove all the custom map merge functions for a generic one:

pub fn merge_iter<M, A, C>(mut prev: M, next: M, _: &C) -> MergeResult<M>
where
    M: Extend<A> + IntoIterator<Item = A>,
{
    prev.extend(next);
    Ok(Some(prev))
}

[JeanMertz]

Hey @milesj I got a few notifications that you're still working on this rewrite, which is great to see!

In case it's of interest to you, I added a bunch of features/changes in a branch I'm maintaining (and using heavily). Most of it is probably not of interest to you or diverges too much from the original intent of schematic, but I wanted to let you know, in case anything is useful to you (even if not the implementation, then perhaps the feature ideas).

[milesj]

Yeah definitely interested with some improvements once this lands. Should make extending the macros much easier.

[JeanMertz]

Should make extending the macros much easier.

Yeah, the macro implementation is quite complex (and scattered) right now. Adding generics support required quite a lot of changes to code all over the place, so hopefully porting that work to the new implementation will be a lot easier (and potentially a good litmus test).

[milesj]

Yeah exactly. Every time I touched this codebase I would be so confused on where everything is lol. I also had the same problem trying to add generics. Shooting for EOY to finish this.