Render updated tags and make generated/modified tags optional for hledger rewrite

[ony]

Note: this PR is based on #1125 and thus includes 2 commits from it.

Features:

• This kinda resolves lib: showTransaction does not show tags in ttags/ptags fields, unless they're also in tcomments/pcomments #664 . Though it is required to use showTransaction . syncTxn in order to get comments updated.
• Late injection of generated/modified tags in comments allows more control from CLIs. Allows to resolve Use hledger-rewrite as a re-factoring tool #1118 .
• Introduced QuickCheck.

New module Hledger.Processing is here for capturing common operations related with post-processing journal or transforming it. Functionality there depends on Hledger.Data and Hledger.Read, so no way to put it inside of Hledger.Data.

Next steps

1. Move date synchronization to syncTxn from TransactionModifier. And ensure we are not adding it many times.
2. (for brave ones) support removing tags and updating dates.

[simonmichael]

Could this be rebased on master, independent of #1125 ? Thanks

[simonmichael]

I did "resolve conflicts" in github which did something confusing to the history in your branch. The feature/render-tags I've just pushed to the main repo is a cleaner rebase on master.

[simonmichael]

Thanks for this, it's interesting. I'm testing and gathering comments slowly.

[simonmichael]

Such as:

+1 for getting us started with property-based testing! hedgehog sounds like an improvement on quickcheck in some ways, did you consider it ?

Another possibility could be to never generate these tags by default, but only when requested with print --rules-tags/rewrite --rules-tags or something. The hidden _ variants could still be generated always, probably.

[ony]

+1 for getting us started with property-based testing! hedgehog sounds like an improvement on quickcheck in some ways, did you consider it ?

I never tried hedgehog before, but it looks like it prefers building generators explicitly rather than "injecting" them via Arbitrary type class. It probably will not integrate that nicely with doctests since it will require you to write something forAll (pair genA genB) $ \ (a, b) -> .... instead of \a b -> ... or \(NonZero a, NonEmpty b) -> ... when you want to have different meaning.

I'm not yet sure how to organize Arbitrary instances in hledger-lib in a way that it will not be noisy in the code and they will not be an orphaned instances.

Another possibility could be to never generate these tags by default, but only when requested with print --rules-tags/rewrite --rules-tags or something.

I'm fine with that. I just thought that this behavior is already around a year here and people could build around it. Changing behavior of print might break their workflow, though I'm not sure which way.

The hidden _ variants could still be generated always, probably.

I think I made it to assume _ variants to be "dynamic" tags that are kinda hidden. It might be better to have them like : variants to ensure that they cannot be produced from parser or explicitly warn in parser about tags that starts with _.

Another possibility could be to never generate these tags by default, but only when requested with print --rules-tags/rewrite --rules-tags or something. The hidden _ variants could still be generated always, probably.

It might be better to generate all tags as dynamic (starting with _) and as result never render them. When we want to render them we can say something like --dynamic-tags/--dynamic-tags=modified to turn tags _modified to modified. Let me know if you believe this direction makes makes more sense than hard-coded list of generated tags to filter out in rewrite.

[simonmichael]

It might be better to generate all tags as dynamic (starting with _) and as result never render them. When we want to render them we can say something like --dynamic-tags/--dynamic-tags=modified to turn tags _modified to modified. Let me know if you believe this direction makes more sense than hard-coded list of generated tags to filter out in rewrite.

Looking for the simplest story internally and externally, I was thinking along the same lines. Eg something like:

"Special hidden tags are added dynamically by hledger in certain situations:

• Transactions generated by periodic transaction rules have a _generated-by: PERIODICRULE tag added
• Transactions modified by auto posting rules have a _modified: tag added
• Postings generated by auto posting rules have a _added-by: AUTOPOSTINGRULE tag added

These dynamic tags are normally not displayed eg by print, but they can still be matched by queries. Eg, you could use tag:_generated-by to match only newly generated periodic transactions, or in an auto posting rule you could use not:tag:_modified to avoid transactions which have already been affected by auto posting rules.

You can use the --show-dynamic-tags flag to make these tags visible (with commands like print, print-unique and rewrite). This may be useful when testing/troubleshooting periodic or auto posting rules, or if you want to permanently record generated transactions'/postings' source in the journal."

I'm assuming the old generated* tags haven't yet been used much and it's ok to change them; this might need a bit more research.

It's hard to know the best design without more experience. If there's to be a special prefix, I favour _, but I'm on the fence about whether a prefix is worthwhile.

I've proposed a different name for your --trace flag because "trace" (and "--debug*") suggest to me showing extra info on stderr just for troubleshooting, while these tags are a bit more than that; also to try and make the purpose more obvious. Other ideas welcome.

[ony]

Just a summary. I'll remove --trace/--debug and add --show-dynamic-tags (though I prefer shorter --dynamic-tags) to all CLIs that print transactions in a way that they can be parsed by hledger again.

I'll remove injection of modified and generated* tags in favor of dynamic versions of them. Thus queries like tag:^modified will not work unless there is such tag in original journal. Though queries like tag:modified and tag:_?modified will still work as before, I guess.

Changing _generated-by to _added-by for "auto-postings" can be done as part of separate PR.

I'll work on this either this evening or in worst case on weekend.

[simonmichael]

Sounds good @ony. I've sent a heads-up/RFC to the mail list.

FYI I think we need different tag names for transactions and postings so that you can query them precisely, since the transaction tags get inherited by postings and vice versa. But I could be wrong, and resolving this as a separate task is fine.

[simonmichael]

Sorry for the delay on this. I haven't been using this feature very recently and I can't seem to get motivated to review it. Could someone else give @ony's latest a code/design review ?

If not, @ony please feel free to merge this.

[simonmichael]

Along with doc updates.

[Xitian9]

Does this need anything other than a rebase to master, or does it still need a code review? I'm happy to do either.

[ony]

Beside code review there were a gentle reminder about updating docs, that I guess, I never followed 😛 .

[simonmichael]

Thanks for pinging this, I think good steps would be:

• rebase
• review current docs/functionality of generated & hidden tags
• document the functional changes (to the above) brought by this PR
• review/test/discuss