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

Errors for default syntax with enabled strict checks #2119

Closed
asyncink opened this issue Dec 1, 2023 · 7 comments
Closed

Errors for default syntax with enabled strict checks #2119

asyncink opened this issue Dec 1, 2023 · 7 comments
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. check

Comments

@asyncink
Copy link

asyncink commented Dec 1, 2023

hi

  1. when enabled strict check for payees, hledger emits errors for all transactions without payee:

    2023-11-30
      Account1  1 USD
      Account2
    
    Strict payee checking is enabled, and
    payee "" has not been declared.
    Consider adding a payee directive. Examples:
    
    payee
    

    this can not be fixed because there is no way to create empty tag

  2. when enabled strict check for tags, hledger emits errors for accounts with implicit type:

    account Assets:Cash  ; type: C
    
    Strict tag checking is enabled, and
    tag "type" has not been declared.
    Consider adding a tag directive. Examples:
    
    tag type
    

    this can be fixed by type tag, but this feels more like temp workaround

p.s. thx for hledger!

@asyncink asyncink added the A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. label Dec 1, 2023
@asyncink
Copy link
Author

asyncink commented Dec 1, 2023

hledger --version
hledger 1.31, mac-x86_64

@simonmichael
Copy link
Owner

Thanks for the report. For 1, perhaps we should allow declaring payee "" ? This would be consistent with commodity. (I'm not sure why just commodity with no "" was not allowed...)

For 2, are you saying that type tag should always be accepted even without a tag declaration because it's one of the hard-coded built-in tag names ?

@asyncink
Copy link
Author

asyncink commented Dec 1, 2023

thank you for fast reply! hledger is already cool, but with the opportunity to discuss its development and remaining errors - it’s amazing.

  1. actually I don’t think that allowing payee "" directive is good enough. according to documentation, user can optionally divide transaction description into two parts with char |: on the left side payee and on the right side and till the end of the line — description. for me it feels consistent that omitting char | is the same that just leaving right side as description without payee:

    2023-10-30 Whole Foods | Groceries for dinner ; <~ it’s payee and description
      Account1 10 USD
      Account2
    
    2023-10-30 Cinema with friends ; <~ it’s just a description (since no payee was provided)
      Account1 10 USD
      Account2
    
    2023-10-30 ; <~ no payee and no decsription
      Account1 10 USD
      Account2
    

    so in my opinion the expected behavior is if strict payees check is fired only for transactions with payee provided (with char ‘|’) and is not firing if no description provided or provided without char ‘|’.

    otherwise (and it’s current behavior for now) enabling strict payees check will parse three transactions above as 3 different payees, but Cinema with friends is just a description. it can be solved like this:

    2023-10-30 | Cinema with friends ; <~ it’s just a description (since to payee was provided)
      Account1 10 USD
      Account2
    

    but then strict payees check will throw error for empty payee. and also users with existing journals have problems with all their transactions.

    to summarize: in my opinion, strict payees check should fire only if character | is provided AND only if left part (substring leftside from the |) is not empty.

  2. are you saying that type tag should always be accepted even without a tag declaration because it's one of the hard-coded built-in tag names — yes, exactly!

@simonmichael
Copy link
Owner

In principle I think payee is "Whole Foods", "", and "" respectively in those three cases, and it should be checked consistently in each case. Because it should be possible to disallow writing an entry without one of the valid payees (as with accounts, commodities, etc.) Possibly some docs/functionality might need to be adjusted here.

@asyncink
Copy link
Author

asyncink commented Dec 2, 2023

"Whole Foods", "", and "" respectively in those three cases

yes, agree

it should be possible to disallow writing an entry without one of the valid payees (as with accounts, commodities, etc.)

yes, got this point, that's right

@simonmichael
Copy link
Owner

I have added payee "" support.

Re automatically declaring the built-in tag names: type is the one you mentioned; t is another one I can think of (generated by timedot's letters notation).

Should it be an error if the user tries to declare these tags themselves with tag type or tag t ?

simonmichael added a commit that referenced this issue Dec 7, 2023
Now using type: in account declarations or generating t: with timedot
letters won't cause the `tags` check to fail.

If a user declares any of these explicitly with a tag directive,
it does not cause an error.
@simonmichael
Copy link
Owner

The built-in tags are now auto-declared, and it's not an error if user accidentally declares them explicitly. I think this is resolved.

simonmichael added a commit that referenced this issue Dec 7, 2023
Now using type: in account declarations or generating t: with timedot
letters won't cause the `tags` check to fail.

If a user declares any of these explicitly with a tag directive,
it does not cause an error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. check
Projects
None yet
Development

No branches or pull requests

2 participants