Generate output

[ThomasdenH]

What is required to generate beancount ouput? Should the provided types just implement Display?

[twilco]

Hi!

Yes, implementing Display on the things in constructs.rs to generate the corresponding Beancount syntax seems like a reasonable option to me. Any thoughts, @mbudde?

[ThomasdenH]

There are some choices to make regarding formatting. A more complex but also more complete solution would be the creation of a configurable type responsible for generating the beancount code. The display could then use that with some reasonable default. But I guess incrementalism is favourable.

I can work on this. Should I base it on the ast-to-constructs branch?

[twilco]

A more complex but also more complete solution would be the creation of a configurable type responsible for generating the beancount code. The display could then use that with some reasonable default.

This indeed sounds good, but I think your plan of starting with a more simple implementation may help us shake out what some useful configurations might be. Thanks for your motivation to work on this! I admittedly haven't had the chance to think much about what the API for output might look like, so I'm excited to see what you come up with.

Should I base it on the ast-to-constructs branch?

Unfortunately, I'm not sure where @mbudde is at with that branch. I think he was headed in a good direction, so basing it off that with the knowledge that it is unfinished might be the best bet.

[mbudde]

I think I would prefer if the formatting code lived in separate crate. That rules out Display but I don't think that is a good option anyway. I have trouble coming up with a use for generating beancount output that doesn't involve some configuration. At minimum I would want the option to configure the alignment column for amounts.

I think we should just go ahead and merge ast-to-constructs into master. It's been a while since I've looked at it but I believe it's in a pretty good shape and it has some changes that would be needed if you were to implement output generation as a separate crate.

[ThomasdenH]

I have implemented basic formatting in this repository: https://github.com/ThomasdenH/beancount_render.

It currently isn't configurable, and hasn't had much testing, but it's a start.

[twilco]

Very nice! I've taken a look and have some high-level thoughts:

1. I see you've created a separate repository and imported this one as a dependency vs. forking this repository and adding render (or something to that effect) as a new directory. That seems like a good plan for faster iteration — long term, though, do you see yourself wanting to merge that into this repo, or keep it separate?
2. Some tests would indeed be useful. Maybe something like an E2E test where we start with a Beancount file, deserialize it into Rust structures and then serialize it back out with the code you have written with the expectation that it's the same before and after. @mbudde created a nice set of tests for parsers.rs that may be worth mimicking.
3. I like that you used thiserror over alternatives that leak into public API of the crate.
4. As you've noted, we'll need to explore some possible configuration points (e.g. @mbudde mentioned account column alignment config above)

We can get into more detailed review-points should you choose to integrate your work into this repository via fork and PR. If you'd prefer to keep your work separate, I wouldn't mind linking to it in the README, although long-term I think having it in one place could make sense.

Seems to be shaping up nicely 👍

[ThomasdenH]

The code is in a separate crate because of the discussion prior in this issue. It would be good to integrate it in here at some point, but I'm fine either way.

I have tried to use these crates in some programs I use for my own finances, and one improvement would be to have more parsing/conversion options for the smaller constructs. For example, for creating an amount, flag or an account from a string or a date from something like NaiveDate from chrono. The default dependencies should be lean, but that would improve usability for me. For this reason it could be good to have actual wrapper types for at least Date and Currency instead of simple type definitions.

[twilco]

The code is in a separate crate because of the discussion prior in this issue. It would be good to integrate it in here at some point, but I'm fine either way.

We should be able to publish multiple different crates from within this repo (https://github.com/twilco/beancount) as long as they are in different sub-directories.

All this aside, thinking about it a bit more, I'm not sure I understand the benefit of making this render/output type code a separate crate, since it doesn't do much standalone. The entrypoint to this render crate:

pub fn render<W: Write>(w: &mut W, document: &Document<'_>)  -> ... {
    ...
}

requires the Document struct from this crate, so consumers would be pulling in both anyways. If anything, I think this render code could simply be a feature gate in the crate we already have.

---

...one improvement would be to have more parsing/conversion options for the smaller constructs...for example, for creating an amount, flag or an account from a string or a date from something like NaiveDate from chrono. For this reason it could be good to have actual wrapper types for at least Date and Currency instead of simple type definitions.

This could be another thing we could feature gate: integration with chrono providing some convenient impl From<beancount::core::Date<'a>> for NaiveDate conversions, and other things of the sort. A wrapper type would be a good idea if we want more functionality beyond basic conversion. Same for Currency.

[mbudde]

Nice work on the formatter, @ThomasdenH.

We should be able to publish multiple different crates from within this repo (https://github.com/twilco/beancount) as long as they are in different sub-directories.

All this aside, thinking about it a bit more, I'm not sure I understand the benefit of making this render/output type code a separate crate, since it doesn't do much standalone.

I have attempted to split up the repo into multiple crates in #24. If having many small crates is too cumbersome to use, an option could be to make a "master crate" that reexports stuff from the other crates. Or we could put everything in the same crate.

...one improvement would be to have more parsing/conversion options for the smaller constructs...for example, for creating an amount, flag or an account from a string or a date from something like NaiveDate from chrono. For this reason it could be good to have actual wrapper types for at least Date and Currency instead of simple type definitions.

I agree!