Remove know-ification of elements due to post state

[CandyAngel]

While it seems intentional, elements (accounts, commodities, payees and tags) becoming "known" because they happen to be in a non-uncleared posting is very surprising (and undocumented?) behaviour. It seems to be not very useful and rather unfitting with the terms "strict" and "pedantic".

This PR removes this "know-ification" and updates the tests for strict and pedantic to match their newly expected output.

[CandyAngel]

I believe this resolves #838, #844 and #846 on Bugzilla.

[jwiegley]

Why is it surprising, exactly?

[CandyAngel]

I just had a look at the documentation again and it's actually only surprising for --pedantic as it isn't mentioned there (but is for --strict).

--pedantic  Accounts, tags or commodities not previously declared will cause errors.

Searching the documentation, I see the --explicit option does the same as this PR and I believe that was present even when I made the original PR. So I guess this PR isn't required, unless you want to also remove that option?

It does seem pointless to say "be pedantic", but then have to say "no, I actually mean it".

Just let me know if you want me to expand the scope of this PR to effectively make --explicit be the default behaviour and remove that option. Otherwise, feel free to close this PR (and sorry for taking your time in that case!).

[CandyAngel]

Oh, it can be surprising with --strict too, depending where in the documentation you read what it does as it says the same as the --pedantic description (just with warnings instead of errors).

EDIT: If you read this section for example.

[jwiegley]

Hmm.. my only worry at this point is how much people may have based their workflow around the current behavior.

[CandyAngel]

I understand the reservation of making such a change. Especially as I tend to be that person with a weird workflow that depends on certain behaviour/implementation details.

If the behaviour isn't changed, the man page really should be. It states that strict/pedantic will cause warnings/errors when entities aren't declared, without noting pending/cleared post state renders them declared (and why would it be expected that it does? You can mis-type an account just as easily when adding a pending/cleared posting as an uncleared one!). This might be why the behaviour is surprising new users.

For what it's worth, I'd make --explicit the default behaviour, deprecate the option (emit a warning when strict/pedantic are used?) and remove it in a few minor versions/next major.

I mean, it would appear the current behaviour is so unintuitive that no-one has pointed out that --explicit gives the behaviour that is expected..

[jwiegley]

Workflow

[501st-alpha1]

I was confused when I first encountered this behavior, and since I discovered --explicit have always used that option with --strict or --pedantic.

I'm in favor of what @CandyAngel suggests, making the default be to ignore the cleared/pending/uncleared status. If there are users who rely on that functionality, I think it'd make more sense to have an option to enable it, rather than an option to disable it.

I likewise agree that if the behavior isn't changed, the documentation should be updated, to make things, ahem, explicit.

[CandyAngel]

Do you want a follow-up PR to emit a message about the changed behaviour when using the involved options and another to remove --explicit to be merged in the future?

[jwiegley]

That would be great, @CandyAngel, thank you!

[tbm]

Didn't --explicit become a no-op with this change? If so, I would not show a message and I would not remove the option (just let it be a no-op).

[tbm]

We definitely need a change to the manual, the man page and doc/NEWS.md though.

[tbm]

Sorry for commenting on this so late. There is one subtle feature about this option which got lost.

In the past, --strict/--pedantic would only produce warnings/errors if you had pre-declared at least one entity of a certain class. So if you had account foo, it would warn about all undefined account names. But if you didnd't have any account declarations at all (doesn't matter for which accounts; an account foo is enough), it would not warn about undefined accounts.

This was a source of confusion for some... why doesn't --strict --explicit complain? Well, you didn't define any accounts.

But you could also argue that it was a feature: it was a way to let ledger warn about account names but not commodities (just don't define any commodities).

Personally, I think it's fine for this to go away... it removes the confusion. But we should also acknowledge that it removes a feature that some people may have relied upon. (But imho if you define accounts, just define commodities as well... there aren't that many anyway. Payees is a different question, but that requires a separate option to be checked anyway).

Example:

2019-01-03 Foo
    Assets:Bank                10.00 EUR
    Assets:Foo

Old ledger: no warnings
New ledger: warns about accounts and commodities

account Assets:Bank

2019-01-03 Foo
    Assets:Bank                10.00 EUR
    Assets:Foo

Old ledger: warns about accounts, but not commodities
New ledger: warns about both

[CandyAngel]

Both those options imply that the strict/pedantic-ness is global. It could be split if people only care about certain aspects e.g. --strict-account, --pedantic-commodities and have strict/pedantic enable all of them.

This would also mean that the options could become more consistent, by adding --strict-payees/--pedantic-payees and aliasing --check-payees to the appropriate option until it is removed.

Could even keep the current old behaviour available and have --strict-auto/--pedantic-auto, where it is enabled on any entity type that has at least one explicit definition.

Actually, the entities to be strict/pedantic about could be arguments to the --strict/--pedantic options e.g. --strict auto --pedantic accounts,payees.

(again, if this behaviour is desired, just let me know and I'll see if I can implement it!)

With regards to --explicit being a no-op, the emitting of the message is to let people know it is a deprecated/no-op so they can remove it as required, before the option itself gets removed in the future. Also, I'll include the changes to those additional places that you listed, thanks for mentioning them! :)

[tbm]

Personally I think that's way too many new options. Like I said, I don't think it makes a great deal for accounts and commodities anyway. It makes a difference for payees but there we have an option already.

However, I think it would be worth to describe the change on the mailing list (along with the subtle changes I mention) and ask if anyone has a problem with it.

Overall I think the new behaviour makes more sense.

With regards to --explicit being a no-op, the emitting of the message is to let people know it is a deprecated/no-op so they can remove it as required, before the option itself gets removed in the future.

Since it's a no-op there is imho no need to remove it ever. Just remove it from the docs. Keep the option so you don't break anything.

[tbm]

@CandyAngel we still need to document this.