account directives should disallow parentheses, square brackets

[simonmichael]

hledger's journal reader accepts but misparses parentheses/square brackets in account directives (reported by dentalfloss). Eg:

account (a)

2022-01-01
    a         0

$ hledger -f a.j accounts
(a)
a
$ hledger -f a.j check accounts
hledger: Error: /Users/simon/src/hledger/a.j:4:
  | 2022-01-01
4 |     a               0
  |     ^

Strict account checking is enabled, and
account "a" has not been declared.

[chrislemaire]

So would the desired behaviour here be the rejection of accounts with parentheses in them? Or would it be that this declaration is translated to an account name of a, so that this example would check to be a well-formed journal?

[chrislemaire]

When looking through the relevant functions, I found this comment explaining the need to be able to parse bracketed accounts. Any account names with special characters really:

-- | Parse an account name, plus one following space if present.
-- Account names have one or more parts separated by the account separator character,
-- and are terminated by two or more spaces (or end of input).
-- Each part is at least one character long, may have single spaces inside it,
-- and starts with a non-whitespace.
-- Note, this means "{account}", "%^!" and ";comment" are all accepted
-- (parent parsers usually prevent/consume the last).
-- It should have required parts to start with an alphanumeric;
-- for now it remains as-is for backwards compatibility.
accountnamep :: TextParser m AccountName
accountnamep = singlespacedtext1p

I believe the parser should then be adjusted here to not parse on surrounding parentheses and square brackets?

[simonmichael]

Yes I think parentheses/brackets in account declarations would ideally be rejected as there's no reason for them. But it might be easier to remove and ignore them instead when parsing account directives. There should be a function for that in AccountName or somewhere.

[simonmichael]

(They are needed when parsing account names in postings, where they have a meaning. )

[simonmichael]

Ahaha. I overlooked that #1987 does not fix this issue. I think account should disallow enclosing parentheses/brackets, ie report an error, not simply ignore them, as they have no purpose in account directives and might cause confusion.

[simonmichael]

Or rather, it does fix the example I gave, making it not fail. But my intention was to reject account (foo) entirely.

[chrislemaire]

Ah I see, well should be an easy enough addition. I'll add a check in the parser in a new PR.

[chrislemaire]

Would you perhaps also like to reject postings such as the following?

2022-01-01
    [(a)]         0

Because of the confusion accidentally nested brackets may cause. If this would be rejected, users are notified of overly bracketed account names, that would otherwise be completely stripped of their brackets later, causing an account name that appears to be different to be shown.

[simonmichael]

Thanks a lot @chrislemaire!

Re rejecting over-enclosed accounts names in postings.. could you show briefly the sequence of events causing confusion ? Preventing that sounds good, hopefully we haven't introduced a new restriction that users will find arbitrary.

(Are you starting to wish "virtual postings" had never been invented ? I am..)