[patch] MultiSearch – an AND-conjugated dynamic filtering of any listbox

[psprint]

Hi.
This time coding style is OK :) I submit a patch that:

1. Adds a new command MultiSearch? which, when invoked:on any listbox:

• adds an input field below the listbox (either shrinks the list or extends the dialog, if there is enough room),
• waits for any input,
• after a character will be entered, it filters the listbox with this string,
• the string is split on spaces and matched one by one, case insensitively,
• all keywords must match for an acceptance of a result.

It also adds a new option multi_search_active_by_default which causes all list boxes to be started with MultiSearch? active. For example, the window list allowing to quickly filter and choose :) Or the file history, syntax chooser, etc.

[andrew_b]

Let's start from https://midnight-commander.org/wiki/Hacking.

[psprint]

make indent

[psprint]

I've added a patch after make indent.

[psprint]

make indent

[psprint]

I've uploaded the correct file now.

[andrew_b]

I think, if you want extend some class for special case, you should create a child class with extended capabilities. Not each input line is required to be assigned with some list box. In you case, the new child class of WInput should be created. Also, new class that extends WLisbox should be created too.

I urge you to read Hacking more carefully, not stopping at Indentation.

---

Some notes about your code.

+    l->list = g_steal_pointer (&l->list_keep);

+    g_auto (GStrv) query_terms = NULL;

g_steal_pointer() and g_auto() are introduced in GLib 2.44. MC requires at least 2.30.

---

+            /* Keys we want others to handle, if not forwarding them to a paired listbox */
+            if (parm == KEY_UP || parm == KEY_DOWN || parm == ESC_CHAR
+                || parm == KEY_F (10) || parm == '\n')
+                return MSG_NOT_HANDLED;

Hardcoded keys should not be used.

---

+            send_message (WIDGET (in->forward_listbox), NULL, MSG_DRAW, 0, NULL);

+    send_message (l, NULL, MSG_DRAW, 0, NULL);
+    send_message (multi_search_in, NULL, MSG_DRAW, 0, NULL);

+        send_message (l, NULL, MSG_DRAW, 0, NULL);

widget_draw()

---

+    return ((WIDGET (l)->state & WST_FILTER) != 0);

widget_get_state()

---

+    in = WIDGET (WIDGET (l)->owner)->find_by_type (WIDGET (WIDGET (l)->owner), input_callback);

widget_find_by_type()

---

+    send_message (w, NULL, MSG_RESIZE, 0, &r_listbox);
+    send_message (owner, NULL, MSG_RESIZE, 0, &r_dialog);

widget_set_size_rect()

---

[psprint]

OK, I agree on the class extending opinion. I'll be working on the patch in the next few days. I have been working on other patch (the TAGS object selection lists) lately.

Other remarks:

• as for the hardcoded keys – I've only left the existing code untouched; should I change it?

• the widget_find_by_type() is IMO not a good choice – I think that the virtual function find_by_type() should be used as it is, as it allows the overloading and automatically directs the search to the most proper function.

[psprint]

I've created two clases:
– WForwardingInput:

typedef struct
{
    WInput base;
    widget_cb_fn base_callback; /* Saved callback of  base class */

    /* Fields for new logic. */
    Widget *forward_to_widget;  /* The paired widget to receive unhandled keys */
} WForwardingInput;

– and WFilteringListbox:

typedef struct 
{
    WListbox base;
    widget_cb_fn base_callback; /* Saved callback of base class */

    /* Fields for new logic. */
    GQueue *list_keep;          /* Unfiltered list (used in  WST_FILTER state). */
    filt_listbox_resize_strategy_t resize_strategy;
} WFilteringListbox;

The two classes cooperate when paired to implement input-query equipped listbox. The pairing is initiated by the listbox which can create the input and extend/make room in dialog. I did run make indent and I've used the file templates.

[psprint]

sub-classing

[andrew_b]

This patch contains patch from #4174. Please cleanup.

If you want some refactoring before adding new feature or fixing bug, you should make refactoring in separated patch.

[psprint]

Yes the string safety patch is included, I've forgot to mention. It would be difficult for me to separate the patch now. Could it stay as it is? As a general cleanup around the completion engine which is the main place where *prev_char are used.

[psprint]

I've fixed all style issues, e.g.: replaced if (p) with if (p != NULL). Also:
– MSG_DRAW → widget_draw,
– I've added a MSG_INIT to WFilteringListbox where I conditionally (ini option) enable MultiSearch? on all listboxes; this way this repeated enabling snippet has been removed from various places where listboxes have been instantinated.

[psprint]

MSG_INIT

[ossi]

It would be difficult for me to separate the patch now.

no, it wouldn't:

• reset to pristine state
• apply first patch
• commit
• overwrite with file state of second patch
• commit

generally, you should familiarize yourself with git rebase -i and git gui (selective staging using the context menu). read ​https://sandofsky.com/workflow/git-workflow/ and maybe some "advanced git" tutorials.

[andrew_b]

Replying to psprint:

I've fixed all style issues

No, you haven't.

, e.g.: replaced if (p) with if (p != NULL).

What about other pointers?
What about integers?

Please reread comment:4.

And I haven't talked in details about your implementation of new widgets yet.

Could you please show some screenshots of what you've done?

[psprint]

Here's an ​Asciinema recording :)

PS. I've forgot to record an useful other feature – using of input's HistoryPrev? and Next commands. It adds another way of quickly moving across the various entries (files, functions, etc.) by repeating of previous searches… Here's a quick ​demo.

[ossi]

the features are undoubtedly cool, though some more polish seems in order:

• the discoverability appears somewhat suboptimal. technically, it's not worse than ctrl-s in the panels, but it might be possible to do better here (where a bit more clutter would be acceptable)
• somewhat related to that, the UI is rather jumpy. don't resize the window.
• are the box drawing glitches in your code or is that just the recording program?

speaking of the panels, using for example ctrl-f (for 'filter') to implement the same there to complement/supersede ctrl-s would be quite awesome (i think there might be an issue open for it). then using the same hotkey in the listboxes would reduce the discoverability issue.

[psprint]

The glitches are in the recording interface.

I think that resizing is needed because it feels more bad to miss an entry because of the list being shrunk than to observe the resize up. I've already made some steps however to limit the jumping, as I've too noticed the unpleasant effect that it has: ​asciicast. Some dialogs are resized up some other aren't and it also depends on the list size.

As for the discoverability, I agree – it would be best if there could be a visible UI element (some sort of a button) added to enable the search mode. The discoverability would then be 100%. What could it be, I wonder, however… Otherwise it would have to be enabled by default, IMO. I was having quiet hopes for this seeing how liberating the feature is.

I attach the hopefully final patch with all the requested changes done (coding style, no string patch, widget_find_by_type, widget_draw, etc. – only MSG_RESIZE is left unchanged as explained in patch).

[andrew_b]

Replying to psprint:

I attach the hopefully final patch with all the requested changes done (coding style, no string patch, widget_find_by_type, widget_draw, etc. – only MSG_RESIZE is left unchanged as explained in patch).

Sighed heavily...
Unfortunately, no.

// comments.
Integer variables that should be gboolean.
Non-explicit checks of pointers and integers.
Non-ASCII symbols.
Etc...

Why do I see all this but you don't?

---

No need in #include <glib-2.0/glib.h>.
<glib.h> is included in lib/global.h.

No need in

widget_cb_fn base_callback; /* Saved callback of base class */

Just make a callback of base class (WListbox) public. input_callback() is public already.

Is it possible to use MSG_NOTIFY instead of new message MSG_UPDATE_LIST?

+    /* Allocate memory for the object body. */
+    object = g_new (WForwardingInput, 1);
+
+    /* Forward the call to construct the inherited object. */
+    base = input_new (y, x, colors, len, text, histname, completion_flags);
+
+    /* Shallow copy and shallow release. */
+    object->base = *base;
+    g_free (base);

Sorry, but it's aufull.

Just create a <base_class>_init() function, make it public and use it in <base_class>_new() as well in <child_class>_new(). See widget_init() for reference.

Likewise for WFilteringListbox.

[psprint]

This time I've really thoroughly checked the source and scanned for an improper style. Changes in this final patch:

– booleans instead of integers where needed,
– direct pointer checks == NULL / != NULL,
– introduced listbox_init() and input_init() functions and used them instead of calling *_new functions,
– updated a few comments and changed into /* … */ (I've only added few comments when quickly updating patch previously),
– used MSG_NOTIFY instead of MSG_UPDATE_LIST,
– extended input's initialization so that it has its history properly read also after later (not at creation) on-the-fly enabling of MultiSearch?,
– added pre_widget_unlink_cb typedef for the pre-unlink function (which unregisters the history events) instead of using generic GDestroyNotify,
– used only ascii symbols instead of the unicode glyphs such as … or · (with a little regrets as they add to the expressiveness of the comments; could this requirement be relaxed a little? to be allowed to use e.g.: ndash –, dot · and ellipsis … ?).

[psprint]

I've added shorthand that I've mentioned earlier:
– if the query includes 'c' or 'h' word, then the listbox is being grepped for filenames ending with ".c" and ".h", respectively,
– if there aren't such files in general in the list, then the letters are grepped normally.

This allows to quickly narrow down the list of files to either source or header… like ​here :) Patch v5.3-2 is attached.

[psprint]

Changes in v5.7:

• to address the discoverability suggestions from ossi: add the option enabling MultiSearch on all listboxes to the main options dialogs in mc and mcedit
• fix a segfault.

I'm constantly using MultiSearch on multiple listboxes (completion, window list, etc.) without problems for 1 month now.

[psprint]

use options dialogs

[andrew_b]

I want you to understand one simple thought: while you hardcode file types in your patches (".c", "h", etc) your patches will not be accepted. No chance. At all.

[ossi]

afaict, what this file extension magic does is saving the user from typing one dot per query (you type "h wid" instead of ".h wid"). that's somewhat underwhelming to put it that way. :D

qt creator's locator, similar to modern web browsers' address bars, supports keywords (single letters, for the most part) at the start of the query to select specific search functions. of course, these mechanism are configurable and/or plugin-based. in any case, this seems rather inappropriate for generic search widgets, and is definitely outside the scope of this issue.