Clean up and document functions --handlers

CHANGELOG.md

@@ -47,6 +47,7 @@ This section is for changes merged to the `major` branch that are not also merge
 - The `jobs` builtin now has a `-q` and `--quiet` option to silence the output.
 - fish now supports `&&`, `||`, and `!` (#4620).
 - The machine hostname, where available, is now exposed as `$hostname` which is now a reserved variable. This drops the dependency on the `hostname` executable (#4422).
+- `functions --handlers` can be used to show event handlers (#4694).
 
 ## Other significant changes
 - Command substitution output is now limited to 10 MB by default (#3822).

doc_src/functions.txt

@@ -39,6 +39,10 @@ You should not assume that only five lines will be written since we may add addi
 
 - `-v` or `--verbose` will make some output more verbose.
 
+- `-H` or `--handlers` will show all event handlers.
+
+- `-t` or `--handlers-type TYPE` will show all event handlers matching the given type
+
 The default behavior of `functions`, when called with no arguments, is to print the names of all defined functions. Unless the `-a` option is given, no functions starting with underscores are not included in the output.
 
 If any non-option parameters are given, the definition of the specified functions are printed.

share/completions/functions.fish

@@ -8,3 +8,5 @@ complete -c functions -s n -l names -d "List the names of the functions, but not
 complete -c functions -s c -l copy -d "Copy the specified function to the specified new name"
 complete -c functions -s D -l details -d "Display information about the function"
 complete -c functions -s v -l verbose -d "Print more output"
+complete -c functions -s H -l handlers -d "Show event handlers"
+complete -c functions -s t -l handlers-type -d "Show event handlers matching the given type" -x -a "signal variable exit job-id generic"

src/builtin_functions.cpp

@@ -324,16 +324,16 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     }
 
     if (opts.handlers) {
-        int type = -1;
+        maybe_t<event_type_t> type_filter;
         if (opts.handlers_type) {
-            type = wcs2event(opts.handlers_type);
-            if (type == -1) {
-                streams.err.append_format(_(L"%ls: Expected generic | variable | signal | exit | job-id for --handlers_type\n"),
+            type_filter = event_type_for_name(opts.handlers_type);
+            if (! type_filter) {
+                streams.err.append_format(_(L"%ls: Expected generic | variable | signal | exit | job-id for --handlers-type\n"),
                         cmd);
                 return STATUS_INVALID_ARGS;
             }
         }
-        event_print(streams, type);
+        event_print(streams, type_filter);
         return STATUS_CMD_OK;
     }
 

src/event.cpp

@@ -50,14 +50,6 @@ static event_list_t s_event_handlers;
 /// List of events that have been sent but have not yet been delivered because they are blocked.
 static event_list_t blocked;
 
-static std::map<int, wcstring> events_map = {
-    {EVENT_SIGNAL, L"signal"},
-    {EVENT_VARIABLE, L"variable"},
-    {EVENT_EXIT, L"exit"},
-    {EVENT_JOB_ID, L"job-id"},
-    {EVENT_GENERIC, L"generic"}
-};
-
 /// Variables (one per signal) set when a signal is observed. This is inspected by a signal handler.
 static volatile bool s_observed_signals[NSIG] = {};
 static void set_signal_observed(int sig, bool val) {
@@ -456,23 +448,41 @@ void event_fire(const event_t *event) {
     }
 }
 
-wcstring event2wcs(int type) {
-    std::map<int, wcstring>::iterator it = events_map.find(type);
+/// Mapping between event type to name.
+/// Note we don't bother to sort this.
+struct event_type_name_t {
+    event_type_t type;
+    const wchar_t *name;
+};
 
-    if (it != events_map.end())
-        return it->second;
+static const event_type_name_t events_mapping[] = {
+    {EVENT_SIGNAL, L"signal"},
+    {EVENT_VARIABLE, L"variable"},
+    {EVENT_EXIT, L"exit"},
+    {EVENT_JOB_ID, L"job-id"},
+    {EVENT_GENERIC, L"generic"}
+};
+
+maybe_t<event_type_t> event_type_for_name(const wcstring &name) {
+    for (const auto &em : events_mapping) {
+        if (name == em.name) {
+            return em.type;
+        }
+    }
+    return none();
+}
+
+static const wchar_t *event_name_for_type(event_type_t type) {
+    for (const auto &em : events_mapping) {
+        if (type == em.type) {
+            return em.name;
+        }
+    }
     return L"";
 }
 
-int wcs2event(wcstring const &event) {
-    for (std::map<int, wcstring>::iterator it = events_map.begin(); it != events_map.end(); ++it) {
-        if (it->second == event)
-            return it->first;
-    }
-    return -1;
-}
 
-void event_print(io_streams_t &streams, int event_type) {
+void event_print(io_streams_t &streams, maybe_t<event_type_t> type_filter) {
     std::vector<shared_ptr<event_t>> tmp = s_event_handlers;
     std::sort(tmp.begin(), tmp.end(),
             [](const shared_ptr<event_t> &e1, const shared_ptr<event_t> &e2) {
@@ -487,39 +497,40 @@ void event_print(io_streams_t &streams, int event_type) {
                         case EVENT_GENERIC:
                             return e1->str_param1 < e2->str_param1;
                     }
-                } else {
-                    return e1->type < e2->type;
                 }
+                return e1->type < e2->type;
             });
 
-    int type = -1;
+    maybe_t<event_type_t> last_type{};
     for (const shared_ptr<event_t> &evt : tmp) {
-        if (event_type == -1 || event_type == evt->type) {
-            if (evt->type != type) {
-                if (type != -1)
-                    streams.out.append(L"\n");
-                type = evt->type;
-                streams.out.append_format(L"Event %ls\n", event2wcs(evt->type).c_str());
-            }
-            switch (evt->type) {
-                case EVENT_SIGNAL:
-                    streams.out.append_format(L"%ls %ls\n", sig2wcs(evt->param1.signal),
-                            evt->function_name.c_str());
-                    break;
-                case EVENT_JOB_ID:
-                    streams.out.append_format(L"%d %ls\n", evt->param1,
-                            evt->function_name.c_str());
-                    break;
-                case EVENT_VARIABLE:
-                case EVENT_GENERIC:
-                    streams.out.append_format(L"%ls %ls\n", evt->str_param1.c_str(),
-                            evt->function_name.c_str());
-                    break;
-                default:
-                    streams.out.append_format(L"%ls\n", evt->function_name.c_str());
-                    break;
+        // If we have a filter, skip events that don't match.
+        if (type_filter && *type_filter != evt->type) {
+            continue;
+        }
 
-            }
+        if (!last_type || *last_type != evt->type) {
+            if (last_type)
+                streams.out.append(L"\n");
+            last_type = static_cast<event_type_t>(evt->type);
+            streams.out.append_format(L"Event %ls\n", event_name_for_type(*last_type));
+        }
+        switch (evt->type) {
+            case EVENT_SIGNAL:
+                streams.out.append_format(L"%ls %ls\n", sig2wcs(evt->param1.signal),
+                        evt->function_name.c_str());
+                break;
+            case EVENT_JOB_ID:
+                streams.out.append_format(L"%d %ls\n", evt->param1,
+                        evt->function_name.c_str());
+                break;
+            case EVENT_VARIABLE:
+            case EVENT_GENERIC:
+                streams.out.append_format(L"%ls %ls\n", evt->str_param1.c_str(),
+                        evt->function_name.c_str());
+                break;
+            default:
+                streams.out.append_format(L"%ls\n", evt->function_name.c_str());
+                break;
         }
     }
 }

src/event.h

@@ -22,7 +22,7 @@
 #define EVENT_ANY_PID 0
 
 /// Enumeration of event types.
-enum {
+enum event_type_t {
     /// Matches any event type (Not always any event, as the function name may limit the choice as
     /// well.
     EVENT_ANY,
@@ -127,15 +127,8 @@ void event_fire(const event_t *event);
 /// May be called from signal handlers
 void event_fire_signal(int signal);
 
-/// Convert a string to the corresponding type
-int wcs2event(wcstring const &event);
-
-/// Convert a type to the corresponding string
-wcstring event2wcs(int type);
-
-/// Called by builtin functions
-/// Dumped all events
-void event_print(io_streams_t &streams, int event_type);
+/// Print all events. If type_filter is not none(), only output events with that type.
+void event_print(io_streams_t &streams, maybe_t<event_type_t> type_filter);
 
 /// Returns a string describing the specified event.
 wcstring event_get_desc(const event_t &e);
@@ -143,4 +136,7 @@ wcstring event_get_desc(const event_t &e);
 /// Fire a generic event with the specified name.
 void event_fire_generic(const wchar_t *name, wcstring_list_t *args = NULL);
 
+/// Return the event type for a given name, or none.
+maybe_t<event_type_t> event_type_for_name(const wcstring &name);
+
 #endif