Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: --forecast=PERIODICEXPR. Fixes #835, #1236 #1250

Merged
merged 1 commit into from Jun 3, 2020

Conversation

adept
Copy link
Collaborator

@adept adept commented Jun 2, 2020

Given that we already have a nice single place where "forecast begin" and "forecast end" is defined, plugging in options that allow overriding one or both of the values is pretty straightforward. I added a test nevertheless

Copy link
Owner

@simonmichael simonmichael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

hledger/Hledger/Cli/CliOptions.hs Outdated Show resolved Hide resolved
tests/forecast.test Show resolved Hide resolved
@simonmichael
Copy link
Owner

simonmichael commented Jun 3, 2020

I'll add more thoughts here, hope it's not too much.

"forecast" or something else ?
As you said last year, "forecast" implies future. Now we're allowing periodic transactions to be generated in the past as well, though that will be less common. We can keep calling it "forecast", I think it's ok. Or we could look for a new name, eg --auto-transactions (and make --auto short for --auto-transactions --auto-postings), or something else; though also with --forecast as an alias.

begin/end or from/to ?
Whatever X we choose above - should the other flags be --X-begin/--X-end, or --X-from/--X-to ? The first is consistent with --begin and --end, helping to show that --begin/--end/--forecast-begin/--forecast-end are the four dates affecting a report. The second reads a little more naturally and grammatically. What's your preference ?

showing future transactions in hledger-ui ?
--forecast has a second function of revealing future-dated transactions in hledger-ui (hidden by default). If we allow any of --forecast, --forecast-begin, --forecast-end to enable periodic transactions, would all of them also do the thing in hledger-ui ? If the "forecast" period is in the past that would be unintuitive.

@adept
Copy link
Collaborator Author

adept commented Jun 3, 2020

I'd like to keep --forecast because supplying PERIODEXPR does not necessary mean that we are talking about the past -- some accounts could be more in the future than the others, and implicitly taking date of the last transaction in the journal as the start of the forecast might imply that we are "going to the future". In this case, using PERIODEXPR means that we simply restore the status quo.

I've just pushed the rework that makes --forecast accept optional PERIODEXPR. When omitted, it will default to current defaults

@adept adept force-pushed the forecast-begin-end branch 2 times, most recently from 7aa9f20 to de97a13 Compare June 3, 2020 21:30
@adept adept changed the title lib: --forecast-begin/--forecast-end. Fixes #835, #1236 lib: --forecast=PERIODICEXPR. Fixes #835, #1236 Jun 3, 2020
@simonmichael simonmichael merged commit b7413ed into simonmichael:master Jun 3, 2020
@simonmichael
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants