functions --hooks #4694
functions --hooks #4694
Conversation
|
This looks nice! Thanks for doing this! I don' t know about the name "hooks," we don't use that term anywhere else. Users cause a function to respond to an event via A few nits I'll put in code review. Also if you get to it you can add this to the CHANGELOG, if not I'll do it. |
src/builtin_functions.cpp
Outdated
| static const struct woption long_options[] = { | ||
| {L"erase", no_argument, NULL, 'e'}, {L"description", required_argument, NULL, 'd'}, | ||
| {L"names", no_argument, NULL, 'n'}, {L"all", no_argument, NULL, 'a'}, | ||
| {L"help", no_argument, NULL, 'h'}, {L"query", no_argument, NULL, 'q'}, | ||
| {L"copy", no_argument, NULL, 'c'}, {L"details", no_argument, NULL, 'D'}, | ||
| {L"verbose", no_argument, NULL, 'v'}, {NULL, 0, NULL, 0}}; | ||
| {L"verbose", no_argument, NULL, 'v'}, {L"hooks", optional_argument, NULL, 'o'}, |
There was a problem hiding this comment.
Seconding @ridiculousfish's comment: "event_handlers" would be more consistent.
|
yeah i prefer it too ; what do you think of it ? |
|
I don't find optional argument really pratical |
src/event.cpp
Outdated
| void event_print(io_streams_t &streams, const wcstring *filter) { | ||
| std::vector<shared_ptr<event_t>> tmp; | ||
|
|
||
| static std::map<int, wcstring> dico = { |
There was a problem hiding this comment.
Please just use a switch statement here (an external function or a lambda):
static const wchar_t *name_for_event_type(int which) {
switch (which) {
case EVENT_ANY: return L"any";
...
}
}
There was a problem hiding this comment.
I also need to do the reverse operation (have a string and get a int)
and for that i can't do a switch statement
src/event.cpp
Outdated
| tmp = s_event_handlers; | ||
| std::sort(tmp.begin(), tmp.end(), | ||
| [](const shared_ptr<event_t> &e1, const shared_ptr<event_t> &e2) { | ||
| if (e1.get()->type == e2.get()->type) { |
There was a problem hiding this comment.
FYI you can write e1->type == e2->type here, no need for get()
src/event.cpp
Outdated
| }); | ||
|
|
||
| int type = -1; | ||
| for (std::vector<shared_ptr<event_t>>::iterator iter = tmp.begin(); |
There was a problem hiding this comment.
for (const shared_ptr<event_t> & evt : tmp) {... is nicer. fish has graduated into C++11 :)
src/event.cpp
Outdated
| static std::map<int, wcstring> dico = { | ||
| {EVENT_ANY, L"any"}, | ||
| {EVENT_SIGNAL, L"signal"}, | ||
| {EVENT_VARIABLE, L"any"}, |
There was a problem hiding this comment.
Looks like a copy-paste error, "any" should be "variable" here.
|
What's the status here? @ridiculousfish: Did you forget about this? |
|
It's been in my dock all this time :/ |
Add a little options to list all events hooks
The current output it's not really pretty but i had no idea to make it better
It's my first contribution don't hesitate to criticize