rename_accounts: Support renaming within all directives, not just Transaction

[ankurdave]

rename_accounts currently only applies to transactions. This causes several problems:

1. After renaming, balance assertions are no longer valid because they still refer to the original account name.
2. The original accounts are still opened, cluttering the chart of accounts.
3. Notes and documents remain attached to the original account name rather than the new account name.

This PR fixes these problems by generalizing rename_accounts to apply to all directives that contain account names, not just transactions.

[redstreet]

Nice upgrade, thanks! If you could please fix the minor linter problems, I'll merge it.

[ankurdave]

Done. flake8 . --count --max-complexity=12 --max-line-length=127 --show-source --statistics passes for me now.

[redstreet]

Thanks, merged!

The one concern I have here is, with this merge, we recreate all directives, not just renamed ones. This has two effects

• performance: rename went from 16ms to 140ms for my data
• debugging might be harder in the future (eg: on upstream changes)

Looking into these more.

[ankurdave]

That makes sense, thanks for benchmarking. I suppose we can check the result of rename_account() and avoid calling _replace() if it's unchanged.

[redstreet]

Agree. Even better, get rename_account() to tell us if there was a change, so we don't have to do an expensive string comparison.

Want to take a dig at it?

[ankurdave]

Sure.