journal: Fully unbracket AccountNames in account directives

[chrislemaire]

Currently an account name like "a:(aa)" will not have (aa) unbracketed. However, this seems reasonable since the full name is unbracketed and thus will not be confused with virtual or virtual-balanced posting.

Resolves #1915

[simonmichael]

Thanks!

I don't love the recursive stripping of all enclosing parentheses/brackets - it looks capable of hiding infinite loop bugs. Eg if textUnbracket changed somehow in future.

Our journal format defines only a single enclosing pair of parentheses or brackets.

Ledger treats [(a)] as a balanced virtual posting to an account named (a).

But I see current hledger already strips multiple enclosing pairs:

1/1
  [[(a)]]  1
  [b]

$ hledger print
2023-01-01
    [a]               1
    [b]

but not perfectly:

1/1
  [([(a)])]  1
  [b]

$ hledger print
2023-01-01
    [(a)]               1
    [b]

Whether we decide to strip one enclosing pair, strip unlimited enclosing pairs, or raise an error when we see more than one enclosing pair, ideally it will be consistent everywhere.

I currently would feel fine with any of these, if we can be sure of not risking infinite loops.

[chrislemaire]

Ah yes, I agree the recursive solution is not the cleanest and could indeed cause some unexpected behaviour later.
I looked into your comment and found that the unexpected behaviour is because of these lines:

concatAccountNames :: [AccountName] -> AccountName
concatAccountNames as = accountNameWithPostingType t $ T.intercalate ":" $ map accountNameWithoutPostingType as
    where t = headDef RegularPosting $ filter (/= RegularPosting) $ map accountNamePostingType as

accountNameWithPostingType and accountNameWithoutPostingType both strip the posting type brackets/parentheses. The former strips types of every part of the account name passed to this function, the latter of the combined account name. Hence the rather strange behaviour of stripping the inner parentheses and outer brackets from [[(a)]]...

To solve your issue with recursion and generalise the unbracketing of account names I changed the textUnbracket function to have the same behaviour as textUnbracketAll had, but non-recursively. (It counts the number of matching left and right brackets and drops from the front and back of the string equally). It looks to me like this will be called every time an account name is parsed from a journal since modifiedaccountnamep calls it indirectly.

It looks like [a:[aa]] is not stripped of its inner brackets because account names are parsed as a whole, not in parts, in the relevant pieces of code. I could still strip the inner brackets if we split up account names before going into concatAccountNames in joinAccountNames, which is called from the parser. Would that be desired?

[simonmichael]

I see the latest behaviour is to strip all outer bracket or parenthesis pairs enclosing the full account name, leaving single brackets/parentheses or inner bracket/parenthesis pairs unchanged.

Practically speaking I think it means users can use brackets or parentheses inside account names, or enclose account name parts other than the top-level part, if they really want to. But they can't enclose a top level part, or a full account name.

The new textUnbracket is clever, I had to read it carefully to understand it.

Looks good to me, I'll merge if you're ready.

[chrislemaire]

I'm ready! Yeah it's too bad textUnbracket is a little more complicated to read now. I couldn't find an easier way of writing it without recursion myself sadly.

[simonmichael]

But it seems nice and safe. Thank you!