Tag/link filtering

[vkazanov]

Hi and hope you have a good day,

This is a first version of tag/link filtering. I use tags and links a lot to extract subsets of my ledger so this is already has value for me personally.

I tried to structure code as nicely as possible, introducing logic in atomic commits. Here's a breakdown of the commits:

1. Introduce function that can show/hide parts of the buffer.
2. A macro that walks all buffer transactions.
3. A set of functions that can hide/show transactions based on provided tag/link names. The functions are used for button actions. Buttons are added to the buffer through font-lock.
4. tests for lower-level functions.

The current public-facing UI makes possible to move the point to a tag and press RET to hide all irrelevant transactions. A mouse click works as well.

It is also possible to directly call (beancount-show-tag "#tag") to restrict view to tagged transactions. (beancount-show-all) cleans filtering.

Further improvements that I feel might be necessary:

1. Better interaction with outline-minor-mode and reveal-mode. Current these do not interact well.
2. Make beancount-tag-show interactive and use completion for tags.
3. Add messages explaining how to remove filtering.

In general, the code can be developed further to support advanced filters, e.g. by date intervals, account names, etc.

What do you think?

Thanks

[vkazanov]

Hm. For some reason the tests do not have the macro defined. I did run all the tests for 4 emacs version locally.

I'll take a look today.

[dnicolodi]

Thanks for working on this. I like the idea of the functionality. I had just a very quick look at the implementation. I'm not familiar with most of the Emacs facilities used to implement this, thus I have not much to offer in terms of code review. However, the code looks clean. Only two important comments.

From what I can tell, this makes the font-lock implementation significantly more expensive, and inherently incompatible with a font-lock implementation based on tree-sitter (on which I'm working on-and-off). I would therefore prefer if this would be off by default. Implementing it as a minor mode is probably the best fit. @monnier What do you think?

I would use the beancount-- prefix for most of the functions introduced here, with the double dash indicating that these are private functions. The current code does not use this naming scheme consistently, and we should fix that.

[vkazanov]

@dnicolodi Thanks!

1. Happy to make this a minor-mode, of course. Something like beancount-button-minor-mode would would make sense.

2. I'll go through the file and introduce more "--"''s in private functions.

3. Performance-wise I didn't notice any hiccups. But this might need a proper assessment. I'll do some profiling on large files to make sure it's not too bad.

4. Also, I found a bug in the previous PR, will submit it separately.

BTW, treesitter facilities would be easy to use for this kind of filtering. But what's interesting is that tree-sitter's parser might be an overkill for such a simple and regular file format as beancount: everything is line-based, indentation is meaningful, no complex expressions.

[monnier]

From what I can tell, this makes the font-lock implementation significantly more expensive,

I can't see why you think it would be significantly more expensive. A rule like (,(concat "\\#[" beancount-tag-chars "]*") . 'beancount-tag) does the same kind of while+re-search-forward with a `put-text-property` inside and I don't expect `make-text-button` to be that much more costly than `put-text-property`.

and inherently incompatible with a font-lock implementation based on tree-sitter (on which I'm working on-and-off).

Can't treesit-font-lock make buttons? You might like to file a bug report requesting that functionality.

I would therefore prefer if this would be off by default. Implementing it as a minor mode is probably the best fit. @monnier What do you think?

No opinion on that part. W.r.t the code, I see various things that would benefit from being tightened, e.g. `beancount-with-every-transaction` duplicates its body argument. It's probably OK in practice given how it's currently used, but it's better to avoid it. I think I'd just do (unless (beancount-inside-transaction-p) (beancount-goto-next-transaction)) before the `while` loop. Also the macro deserves a `(debug t)` declaration and should use (indent 0) rather than (indent 1), since (indent 1) means that the first arg is indented differently from the rest (like we do for `prog1` or `with-current-buffer`). Also, I'd use a single button kind (and command) for both which should help reduce the code duplication. Stefan

[vkazanov]

@dnicolodi I've taken both your and @monnier 's code-level comments into account in everything but the minor mode question.

Here's the status quo:

1. beancount-show can filter out transactions based on autocompleted link/tag names.
2. beancount-show-all removes all filtering
3. filtering automagically works when the user clicks on links and tags

What I can do is provide something a minor mode that would introduce clickable links and tags (and maybe more things in the future).

What do you think?

[monnier]

No opinion on the minor mode, so AFAICT the code looks OK (tho you might want to "rework" the set of patches into a cleaner patchset). Here are just thoughts that crossed my head, probably for future development: - Not sure I find it "natural" for a click on a tag/link to hide all the other transactions. I can't think offhand of more a useful operation, so it's probably fine, tho. - Emacs being the inventor of the incremental search, I'd expect `M-x beancount-show` to filter the transactions incrementally as I type the tag/link name. 🙂 - I can't think of any reason why `beancount-show` should limit itself to tags/links: we could let it match any other part of a transaction.

[dnicolodi]

tho you might want to "rework" the set of patches into a cleaner patchset

Yes, please 🙂

Not sure I find it "natural" for a click on a tag/link to hide all the other transactions. I can't think offhand of more a useful operation, so it's probably fine, tho.

This is one of the reasons why I see this addition good for a minor mode and less suitable for default behavior.

[dnicolodi]

This and the next hunk are bug fixes for the last PR. Please submit these as a separate PR with a proper commit message.

[vkazanov]

Agree. I'll isolate these into a separate branch for now. This code definitely needs more unit tests.

[vkazanov]

@dnicolodi , I've cleaned up the patchset to restructure changes accumulated so far.

But I respectfully disagree about user expectations around clicking on tags and links. Think Fava (or any other tools that utilise a concept of tags). If somebody sees a tag and clicks it - what should happen? Fava shows all transactions involving the tag clicked. Wordpress does this. Github does that as well.

And this is exactly what I have as my personal use case for the feature. So I have lots of transactions, and some of them are marked as "#trip" and "#torquay-may-2024", which means its a family trip to Torquay, UK, and is also one of the trips that happened this year . Sometimes I want to tweak postings involved in that particular event, sometimes I just want to see how things look in general over time.

Clicking/pressing a tag is a very natural way to just to get to an overview of things.

With only basic font-lock facilities in place I would have to either do isearch or grep, and keep jumping around the buffer. Careful outline organisation gives the higher level structure to the buffer but does not help with this requirement.

(beancount-show) with tag/link completion is a continuation of this line of thought. What if I am not sure which tags/links are available? What is available? What do I want to see?

Anyway, you are the maintainer, I am just a happy user and an occasional contributor looking to support my use case. :-)

[vkazanov]

@monnier with this patchset introducing basic hiding mechanisms, I have something better in mind: stacking filters.

Here's my typical investigation:

1. I want to see all transactions related to a given account.
2. Then I need to see everything about the account within a certain timeframe.
3. Limit transactions to a tag.

This means I want to my filters to stack! But maybe that's a bit too ambitious for now.

On incremental search and not limiting the search to tags/links. What do mean? A substring search? Regexp search? Something smarter?

Thanks

[monnier]

On incremental search and not limiting the search to tags/links. What do mean? A substring search?

AFAICT currently you do substring search (with completion to enter tag names and and link names) tho limited to the first line of each transaction. So I'm suggesting to make it "official" (so you can filter for names in the "..." comments) and maybe to search the rest of the transaction as well (e.g. to filter for account names).

Regexp search?

It would be easy to generalize to regexps, but I don't know how important that would be (it's mostly an issue of UI). Stefan

[vkazanov]

@dnicolodi ,

So I've split this branch into 2 sets of changes:

1. Functions providing basic tag/link search facilities. We can merge things as is right now, or I can expand slightly to support the more general case @monnier suggested.
2. Clickable tags/link - I'll make this into a minor mode and make a separate PR.

PS fixes were extracted into #52 and #53

[blais]

I merge main just now but there remains an error.

lumia [git|main]:~/tmp/vkazanov/beancount-mode$ make compile
emacs -Q -batch -f batch-byte-compile beancount.el

In beancount-show:
beancount.el:1313:37: Warning: reference to free variable ‘beancount-tag-or-link-regexp’

Please fix and I'll merge this PR.