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

Regular-expression limited depth limits #2292

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Xitian9
Copy link
Collaborator

@Xitian9 Xitian9 commented Nov 23, 2024

Previously depth-limiting was universal across all accounts, e.g. all
accounts are clipped to depth 2. However, sometimes you want certain
accounts clipped to a different depth than others, e.g. all expenses to
depth 3, while all assets to depth 2. This commit enables depth-limiting
to optionally include a regular expression, which limits the accounts it
applies to.

More than one depth limit can be passed, and they are applied to each
account name by the following rules:

  • If one or more regular-expression depth limit applies, use the
    smallest one
  • If no regular-expression depth limits apply, and a flat depth limit is
    supplied, use that
  • Otherwise, do not do any depth limiting

For example, this will clip all accounts matching "assets" to depth 3,
all accounts matching "expenses" to depth 2, and all other accounts to
depth 1.
--depth assets=3 --depth expenses=2 --depth 1

@Xitian9
Copy link
Collaborator Author

Xitian9 commented Nov 24, 2024

There are still some parts of the design space to be decided on here. As implemented, then if more than one regular expression matches a given account name, then the most aggressive depth clipping will be done (i.e. the minimum of the provided depths).

A more advanced method would be to choose the most specific regular expression and use that depth. I didn't originally implement it this way because I didn't see a straightforward way to evaluate which regular expression was more specific, but I think the following algorithm may work:

The specificity of a match is the first place in the chain of parents of the account where the regular expression starts matching. So for assets:bank:savings, the specificity of the regular expression assets would be 1, as it starts matching right at the first level. On the other hand, the specificity of the regular expression savings (or assets:bank:savings) would be 3, as this doesn't start matching until you get three levels in.

The proposed algorithm would then be to apply the depth limit for the most specific match. Or take the later if two regexps have the same specificity: we can't get around tie breakers entirely.

@Xitian9
Copy link
Collaborator Author

Xitian9 commented Nov 24, 2024

I think the modified proposal is an unambiguous win, so I've made the change. Let me know if you disagree.

Previously depth-limiting was universal across all accounts, e.g. all
accounts are clipped to depth 2. However, sometimes you want certain
accounts clipped to a different depth than others, e.g. all expenses to
depth 3, while all assets to depth 2. This commit enables depth-limiting
to optionally include a regular expression, which limits the accounts it
applies to.

More than one depth limit can be passed, and they are applied to each
account name by the following rules:
- If one or more regular-expression depth limit applies, use the
  most specific one
- If no regular-expression depth limits apply, and a flat depth limit is
  supplied, use that
- Otherwise, do not do any depth limiting

For example, this will clip all accounts matching "assets" to depth 3,
all accounts matching "expenses" to depth 2, and all other accounts to
depth 1.
--depth assets=3 --depth expenses=2 --depth 1
The regular expression depths are ignored, and only the flat depths are
used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant