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

matching, tags, comments, scope confusion with hledger rewrite / hledger print --auto #745

Open
msmart opened this Issue Apr 26, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@msmart

msmart commented Apr 26, 2018

I'm trying to use hledger rewrite to rewrite postings which match certain tags combinations. I would like to run hledger rewrite several times without adding additional rewrites.

To this end, I thought I can add the tag "rewritten" to the rewritten posting and ignore them in the query of the rewrite rule. But in my environment this is not working as I would expect it to.

Given the following sample.ledger file:

2018/4/1 * test
    ; details:beer
    a  2
    b 

2018/4/1 * another test
    ; details:food
    a  20
    b 

and the following sample.rewrite file

= a tag:details=beer not:tag:rewritten
    a  *-1 ; rewritten:
    c  *1 

The hledger rewrite -f sample.journal -f sample.rewrite| hledger print -f - gives:

2018/04/01 * test     
    ; details:beer    
    a               2 
    b                 
    a              -2    ; rewritten:        
    c               2 

2018/04/01 * another test                    
    ; details:food    
    a              20 
    b                 

However, hledger rewrite -f sample.journal -f sample.rewrite|hledger print -f -|hledger rewrite -f - -f sample.rewrite gives

2018/04/01 * test
    ; details:beer
    a               2
    b
    a              -2    ; rewritten:
    c               2
    a              -2    ; rewritten:
    c               2

2018/04/01 * another test
    ; details:food
    a              20
    b

I would have expected that the posting with details:beer and tag:rewritten would not match a second time.

Am I missing something here?

PS: Thanks to all involved in the development of hledger from a happy user.

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Apr 26, 2018

Thanks for the report. That sounds like a reasonable expectation, and it would be a welcome enhancement for the rewrite command.

I have never used rewrite myself. IIRC it is a more featureful alternative to the built-in http://hledger.org/manual.html#automated-postings. Could those work for you/do they work any better ?

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Apr 26, 2018

Cf http://hledger.org/manual.html#rewrite and the more informative hledger rewrite -h. It seems to be simply another way to invoke automated postings (instead of --auto), with the ability to provide rules on the command line (--add-posting) and to show a diff (--diff).

@simonmichael simonmichael added the docs label Apr 26, 2018

@ony

This comment has been minimized.

Collaborator

ony commented Apr 27, 2018

Tag rewritten is applied to particular posting:

2018/04/01 * test     
    ; details:beer    
    a               2   ; <-- will be targeted again since it matches
    b                 
    a              -2    ; rewritten:        
    c               2 

If you would write:

= a tag:details=beer not:tag:rewritten
    ; rewritten:
    a  *-1
    c  *1 

Then you may expect it to be applied to transaction level and avoid rewriting whole transaction.

But it should be said somewhere in docs that = marks "automated postings to existing transactions". I.e. it creates new postings based on matched ones and have no other effect on whole transaction.

@msmart

This comment has been minimized.

msmart commented Apr 27, 2018

thanks for the quick feedback.

@ony I've updated the sample.rewrite file to look like this (your comment overlapped so I edited this comment to reflect your input):

= a tag:details=beer not:tag:rewritten
    ; rewritten:
    a  *-1  
    c  *1 

Then, I've tried using automated postings with the --auto option and the files mentioned above on hledger version 1.9 with the following command:

hledger -f sample.journal -f sample.rewrite print --auto|hledger -f - -f sample.rewrite print --auto

But this gives the following result:

2018/04/01 * test
    ; details:beer
    a                          2
    b
    ; rewritten:
    a                         -2
    c                          2
    ; rewritten:
    a                         -2
    c                          2
    ; rewritten:
    a                          2
    c                         -2

2018/04/01 * another test
    ; details:food
    a              20
    b

I'm unsure whether I should expect this.

On a side note: When removing the account name from the sample.rewrite file like this:

= tag:details=beer not:tag:rewritten
    a  *-1  
    c  *1 
hledger -f sample.journal -f sample.rewrite print --auto

gives:

2018/04/01 * test
    ; details:beer
    a               2
    b
    a              -2
    c               2
    a               2
    c              -2

2018/04/01 * another test
    ; details:food
    a              20
    b

I would not expected this output as well. Possibly, a good place to start debugging.

For the time being, I'll change my workflow to side step the issue.

Thanks again.

@ony

This comment has been minimized.

Collaborator

ony commented Apr 28, 2018

I would not expected this output as well. Possibly, a good place to start debugging.

Definitely, adding comment to the last posting in transaction of matched posting is unexpected and useless behavior that should be changed.

@msmart , @simonmichael , since issue is not matching but adding comments what you think about changing title of this issue to something like "Transaction-level comments in automated postings"?

Let's also clarify some ambiguity.

= a tag:details=beer not:tag:rewritten
    ; rewritten:
    a  *-1  
    c  *1

= c
    ; :c-rewritten:
    c  *-1  
    d  *1

2018/4/1 * test
    ; details:beer
    a  3  ; <-- first match
    a  2  ; <-- second match
    b

In this case we have two postings that may trigger adding rewritten tag, but once one of them trigger automated posting it renders other one out of the match. Potential results

2018/4/1 * test
    ; details:beer
    ; rewritten:
    ; :c-rewritten:
    a  3  ; <-- triggered first automated posting
    a  2  ; <-- skipped because transaction already have tag "rewritten"
    b
    a  -3
    c  3  ; <-- triggered second automated posting
    c  -3  
    d  3

If we'll change current behavior and don't user result of previous automated postings to apply next ones (already quiet broken) then it can be like:

2018/4/1 * test
    ; details:beer
    ; rewritten:
    a  3  ; <-- triggered based on original transaction
    a  2  ; <-- triggered based on original transaction that have no tag "rewritten"
    b
    a  -3
    c  3  ; <-- skipped since this posting is not part of original transaction

@simonmichael, what you think about dropping feature of adding automated postings for automated postings?

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Apr 28, 2018

All sounds good @ony. If somebody needs to add postings in several stages, they can pipeline multiple commands.

@msmart msmart changed the title from hledger rewrite ignores negative match to Transaction-level comments in automated postings Apr 30, 2018

@simonmichael simonmichael changed the title from Transaction-level comments in automated postings to matching, tags, comments, scope confusion with hledger rewrite / hledger print --auto Jul 26, 2018

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Jul 26, 2018

This is quite hard to follow and requires careful reading. More notes:

  • Re ony's comment suggesting adding a transaction comment to the rule. As of hledger-1.10, such a comment is misparsed as another posting (the semicolon has no effect) and you may get an error about a missing amount.

  • Another difference between rewrite and print --auto: rewrite combines all files into one journal before running rules, so rules in one file affect all files. print follows the usual directive scope rules, so rules in one file do not affect the other (sibling/parent) files. As of hledger 1.10 at least.

  • It's not great that rewrite and print --auto do almost the same thing, and in slightly different ways. I have intended for rewrite to grow and be able to do more complex things (like rewriting and removing postings). An simpler fix would be to just drop it.

  • I'm not able to reproduce either result in msmart's last comment, with hledger 1.9 or 1.10.

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Jul 26, 2018

  • rewrite QUERY can apply rules selectively only to the (matched postings in) transactions matched by QUERY (but still prints all transactions). print --auto QUERY applies rules to the postings in all transactions, then prints only the transactions matched by QUERY.

simonmichael added a commit that referenced this issue Jul 26, 2018

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Jul 26, 2018

In short: I've updated rewrite's help, which now says:

  Print all transactions, rewriting the postings of matched transactions.
  For now the only rewrite available is adding new postings, like print --auto.
...
#### rewrite vs. print --auto

This command predates print --auto, and currently does much the same thing,
but with these differences:

- with multiple files, rewrite lets rules in any file affect all other files.
  print --auto uses standard directive scoping; rules affect only child files.

- rewrite's query limits which transactions can be rewritten; all are printed.
  print --auto's query limits which transactions are printed.

- rewrite applies rules specified on command line or in the journal.
  print --auto applies rules specified in the journal.

And, I'm not currently seeing any bug here, aside from the confusing overlap of rewrite and print --auto.

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Jul 26, 2018

Oh, and the misparsing of a "transaction comment" inside an automated posting rule.

simonmichael added a commit that referenced this issue Jul 26, 2018

journal: ignore transaction comments in auto posting rules (#745)
Previously they were misparsed as account names.

simonmichael added a commit that referenced this issue Jul 26, 2018

journal: fix breakage in auto posting rule parser (#745)
I was negligent and did not test enough. This should ignore
transaction comments in auto posting rules more safely.
It also adds support for trailing comments on the first line of auto
posting rules, which previously were misparsed as part of the query.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment