add sidebar search

[lucilanga]

• What does this PR do?

As title states it will add search option for sidebar mailboxes

• Screenshots (if relevant)
  add search possibility to sidebar #2397 (comment)

• What are the relevant issue numbers?
  add search possibility to sidebar #2397

A couple of comments here:

Right of the bat I had to drag sidebar/private.h into enter/window.c. I know this is bad, but being new at this I don't know how to properly lookup sidebar window to pass it to the sb_function_diapatcher(). I've tried several ways to look for it, but with no success, so I dropped the include, passed d->win to the function and called it done.

I had a miserable time adding SidebarDefaultBindings, not because the API wasn't clear or so but because of several
(mtype != MENU_EDITOR) like instances in the code that prevented it from working - we really should get rid of those, I had to put mtype == or mtype != MENU_SIDEBAR in five places, I'm not even sure every instance is required.

search operation is called OP_SIDEBAR_START_SEARCH as in the matcher, I figured out I keep this name because I might later do the search/limit thing that I initially wanted, so keep the namespace open for OP_SIDEBAR_SEARCH OP_SIDEBAR_LIMIT.

I've cheated on the MENU_SIDEBAR ops, I do handle OP_EDITOR_BACKSPACE on MENU_SIDEBAR only to pass it to the enter_function_dispatcher(). The only thing I need there is the callback support for delete buffer operation so I didn't feel like doing editor_backspace code all over again in the sidebar. Perhaps I should put a comment there before anyone thinks that's a typo.

[flatcap]

Well done!

I've pushed a few fixes, 7a46b60, so that all the automated tests build and run correctly.

The only significant change was to pass the Sidebar's Window to the callback, rather than its Private Data.
This means we avoid a big dependency problem.

I'll have a closer look through the code in the morning.

[flatcap]

I've had a good look at the PR and I see the problem (with the existing code).

The code that reads the keyboard is also doing the lookup from, say, d to OP_DELETE.
That sort of logic should be done at a much higher level -- something I've considered before, but not got around to.

The rough idea, for the Index, would be:

• mutt_getch() returns <Ctrl-O>
• Try: index_function_dispatcher()
  • Lookup <Ctrl-O> -- No Match
• Try: sidebar_function_dispacher()
  • Lookup <Ctrl-O> -- Match: OP_SIDEBAR_OPEN, op_sidebar_open()
  • Execute function
• Repeat

I'm still investigating how much work it is to fix properly; I'll post my findings when I'm done.

[flatcap]

Years ago, I created some diagrams in the gfx repo.
Diagrams 1 and 5 show some of my ideas about the "current" and the "ideal" behaviour.

• https://raw.githubusercontent.com/neomutt/gfx/main/seq/1-func-index-pager.svg
• https://raw.githubusercontent.com/neomutt/gfx/main/seq/5-event-multikey.svg

[flatcap]

#3916 - What I understand about key bindings.
I need to kill some of the business logic in km_dokey_event(), possibly starting with the lookup:

struct Keymap *map = STAILQ_FIRST(&Keymaps[mtype]);

[lucilanga]

So I've been working on the paging thing I've mentioned earlier in the bug report.
As we type in the pattern, the overlay gets sorter but paging/framing remains the same.
and it messes UP/DOWN scrolling.

It seems I cannot use top_index or bot_index because sb_recalc() rewrites them anyway.
So what I could do instead is add a new flag on SidebarWindowData to ask sb_recalc() to scan for the whole framing interval. We do exactly the same thing if sidebar_new_mail_only or sidebar_non_empty_mailbox variables are set.

I suppose I could do a separate recalc() function just for the overlay. We might need something similar in the future, if we decide to do more this quick/overlay stuff.
But I think an overlay recalc would be overlay overkill :) so obviously I prefer the former for now.

[flatcap]

As we type in the pattern, the overlay gets shorter but paging/framing remains
the same. and it messes UP/DOWN scrolling.

When I refactored the Menu code, I drew a diagram.
A long list of items with a view (window) superimposed.

I listed all the possible start conditions, from an empty list, to overflowing the view at both ends.

Next, I listed all the possible actions that could be performed on each of the lists and what I'd like the perfect result to be.
For the Menu there were ops that moved the selection and ops that moved the view.

I cannot use top_index or bot_index because sb_recalc() rewrites them
I could do instead is add a new flag on SidebarWindowData ...

OK
It may be more work to refactor the code, but I'm confident the result will be much simpler.

I think an overlay recalc would be overlay overkill :)
so obviously I prefer the former for now.

OK. I'm always around on IRC if you want to discuss ideas.

[flatcap]

Meanwhile, I'm still investigating how buf_get_field() and km_dokey_event() interact.

km_dokey_event() has a lot of business logic that we don't need.
Most callers need to fall back to the Generic menu (retry_generic()).
Some of that macro code can probably go and all that Imap **** needs factoring out.

I'll probably end up with a parallel buf_get_field2() and km_dokey_event2().

[flatcap]

I've been doing some renaming to the NeoMutt code.
I've rebased your commits over main and squashed my previous fixes into your commit.

The only change that affects you, is that I've renamed buf_get_field() to mw_get_field().
The interactive functions that use the Message Window are now prefixed with mw_.

[lucilanga]

.. and dlg_change_folder() now index_change_folder() . And yes, I've seen your commits and I do like the changes!

[lucilanga]

When I refactored the Menu code, I drew a diagram. A long list of items with a view (window) superimposed.

it didn't make it in the gfx repo, did it ?
I've tried searching bug reports but I'm terrible at finding things on GH.

I'll probably end up with a parallel buf_get_field2() and km_dokey_event2().

Perhaps you would like me to look into this. I know my 1000+ commits on main :) do not qualify. But I feel somehow
responsible for triggering this. I know it has been on your plate for some time but I'm the idiot who brought it up :)
Maybe I can take off some grunt work from you on this.

[flatcap]

I drew a diagram

it didn't make it in the gfx repo, did it ?

Sorry, I think it may only have been on paper.

I'll probably end up with a parallel buf_get_field2() and km_dokey_event2().

Perhaps you would like me to look into this.
Maybe I can take off some grunt work from you on this.

Yes, PLEASE ❤️

I'm a programmer, not a manager, so I have a hard time delegating coding :-)

---

Some final thoughts...

Each Dialog has set of mappings that are stored in Keymaps[MENU_X].
Most Dialogs also fallback to Keymaps[MENU_GENERIC].

If we can pass multiple Keymap arrays into km_dokey_event(), then we can eliminate some of the logic (and retry_generic()).

The other MENU_EDITOR logic is to hide timeouts.
It's convenient, but we can probably move that out to mw_get_field()

[flatcap]

Rebased over main