Should prepare actually do validation be able to return
CMD_RETURN_ERROR? Or would we want to run hooks even when we know the
command is going to fail?
I think struct cmd_ctx would be better than cmd_context, it matches all
the others.
Could struct cmd_q just contain a cmd_ctx rather than a pointer?
Particularly since it appears to be leaked :-).
Also I would be inclined to declare it as:
struct cmd_ctx {
struct client *c;
struct window *w;
struct window *w2;
struct window_pane *wp;
struct window_pane *wp2;
...
}
Some other stuff, mostly minor apart from cmd-queue.c. Trimmed out a lot
of context hope it is clear enough:
> +void
> +cmd_bind_key_prepare(unused struct cmd *self, unused struct cmd_q *cmdq)
> +{
> + return;
> +}
This return is unnecessary.
> +void
> +cmd_break_pane_prepare(struct cmd *self, struct cmd_q *cmdq)
> +{
> + struct args *args = self->args;
> + struct cmd_context *cmd_ctx = cmdq->cmd_ctx;
> +
> + cmd_ctx->wl = cmd_find_pane(cmdq, args_get(args, 't'),
> + &cmd_ctx->session, &cmd_ctx->wp);
Spacing looks funny.
> +void
> +cmd_display_message_prepare(struct cmd *self, struct cmd_q *cmdq)
> +{
> + struct args *args = self->args;
> + struct cmd_context *cmd_ctx = cmdq->cmd_ctx;
> +
> + if (args_has(args, 't'))
> + cmd_ctx->wl = cmd_find_pane(cmdq, args_get(args, 't'),
> + &cmd_ctx->session, &cmd_ctx->wp);
Please {}s around multiline compound statements.
> + else
> + cmd_ctx->wl = cmd_find_pane(cmdq, NULL, &cmd_ctx->session,
> + &cmd_ctx->wp);
In cmdq.c:
> + /*
> + * If we set no session via this---or the prepare()
> function
> + * wasn't defined, then use the global hooks, otherwise
> used
> + * the intended session's hooks when running the
> command.
> + */
> + hooks = (cmdq->cmd_ctx->session != NULL) ?
> + &cmdq->cmd_ctx->session->hooks : &global_hooks;
This would be much easier to read as an if () not a ?:.
> +
> + /*
> + * For the given command in this list, look to see if
> + * this has any hooks.
> + */
> + xasprintf(&hook_before_name, "before-%s",
> cmdq->cmd->entry->name);
> + xasprintf(&hook_after_name, "after-%s",
> cmdq->cmd->entry->name);
> + hook_before = hooks_find(hooks, hook_before_name);
> + hook_after = hooks_find(hooks, hook_after_name);
> +
> + free(hook_before_name);
> + free(hook_after_name);
I think I would add a helper something like:
void
cmdq_run_hook(struct hooks *hooks, const char *prefix, struct cmd *cmd,
struct cmdq *cmdq)
{
struct hook *h;
char *s;
xasprintf(&s, "%s-%s", prefix, cmd->entry->name);
if ((h = hooks_find(hooks, s)) != NULL)
hooks_run(h, cmdq);
free(s);
}
Then the changes to cmdq_continue could be much smaller.
> +
> cmd_print(cmdq->cmd, s, sizeof s);
> log_debug("cmdq %p: %s (client %d)", cmdq, s,
> - cmdq->client != NULL ? cmdq->client->ibuf.fd : -1);
> + cmdq->client != NULL ?
> cmdq->client->ibuf.fd : -1);
>
> cmdq->time = time(NULL);
> cmdq->number++;
>
> guard = cmdq_guard(cmdq, "begin");
> +
> + /* Run hooks before the command, the command itself,
> + * and then any hooks after it. At the moment, hooks
> + * do not incur return value checking, and hence do
> + * not invalidate the actual command from running
> + * should a hook fail to do so.
> + */
> + hooks_run(hook_before, cmdq);
Eg this would just become cmdq_run_hook(hooks, "before", cmdq->cmd, cmdq).
> retval = cmdq->cmd->entry->exec(cmdq->cmd, cmdq);
> + hooks_run(hook_after, cmdq);
> +
> if (guard) {
> if (retval == CMD_RETURN_ERROR)
> cmdq_guard(cmdq, "error");
> +enum cmd_retval
> +cmd_set_hook_exec(struct cmd *self, struct cmd_q *cmdq)
> +{
> + struct args *args = self->args;
> + struct session *s;
> + struct cmd_list *cmdlist;
> + struct hooks *hooks_ent;
> + struct hook *hook;
> + char *cause;
> + const char *hook_name, *hook_cmd;
> +
> + if (args_has(args, 't'))
> + if ((s = cmdq->cmd_ctx->session) == NULL)
> + return (CMD_RETURN_ERROR);
> +
> + if (s == NULL && cmdq->client != NULL)
> + s = cmdq->client->session;
> +
> + if ((hook_name = args_get(args, 'n')) == NULL) {
> + cmdq_error(cmdq, "No hook name given.");
Errors should not have a leading capital letter and no period.
> + return (CMD_RETURN_ERROR);
> + }
> +
> + hooks_ent = args_has(args, 'g') ? &global_hooks : &s->hooks;
> +
> + if (s != NULL && args_has(args, 'u')) {
> + hook = hooks_find(hooks_ent, (char *)hook_name);
> + hooks_remove(hooks_ent, hook);
> + return (CMD_RETURN_NORMAL);
> + }
> +
> + if (args->argc == 0) {
> + cmdq_error(cmdq, "No command for hook '%s' given.", hook_name);
Here too.
> + return (CMD_RETURN_ERROR);
> + }
> + hook_cmd = args->argv[0];
> +
> + if (cmd_string_parse(hook_cmd, &cmdlist, NULL, 0, &cause) != 0) {
> + if (cmdlist == NULL || cause != NULL) {
> + log_debug("Hook error: (%s)", cause);
> + cmdq_error(cmdq, "Hook error: (%s)", cause);
And here. Also can this log go away now?
> + return (CMD_RETURN_ERROR);
> + }
> + }
> +
> + if (cmdlist == NULL)
> + return (CMD_RETURN_ERROR);
> +
> + hooks_add(hooks_ent, hook_name, cmdlist);
> + return (CMD_RETURN_NORMAL);
> +}
> diff --git a/cmd-show-hooks.c b/cmd-show-hooks.c
> new file mode 100644
> index 0000000..c27f6d7
> --- /dev/null
> +++ b/cmd-show-hooks.c
> @@ -0,0 +1,72 @@
> +/* $Id$ */
> +
> +/*
> + * Copyright (c) 2012 Thomas Adam <tho...@xteddy.org>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF MIND, USE, DATA OR PROFITS, WHETHER
> + * IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING
> + * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/types.h>
> +
> +#include <ctype.h>
> +#include <stdlib.h>
> +
> +#include <string.h>
> +
> +#include "tmux.h"
> +
> +enum cmd_retval cmd_show_hooks_exec(struct cmd *, struct cmd_q *);
> +void cmd_show_hooks_prepare(struct cmd *, struct cmd_q *);
> +
> +const struct cmd_entry cmd_show_hooks_entry = {
> + "show-hooks", NULL,
> + "gt:", 0, 1,
> + "[-g] " CMD_TARGET_SESSION_USAGE,
> + 0,
> + NULL,
> + NULL,
> + cmd_show_hooks_exec,
> + cmd_show_hooks_prepare
> +};
> +
> +void
> +cmd_show_hooks_prepare(struct cmd *self, struct cmd_q *cmdq)
> +{
> + struct args *args = self->args;
> + struct cmd_context *cmd_ctx = cmdq->cmd_ctx;
> +
> + cmd_ctx->session = cmd_find_session(cmdq, args_get(args, 't'), 0);
> +}
> +
> +enum cmd_retval
> +cmd_show_hooks_exec(struct cmd *self, struct cmd_q *cmdq)
> +{
> + struct args *args = self->args;
> + struct session *s;
> + struct hook *hook;
> + struct hooks *hooks_ent;
Why not just hooks?
> + char tmp[BUFSIZ];
> + size_t used;
> +
> + if ((s = cmdq->cmd_ctx->session) == NULL)
> + return (CMD_RETURN_ERROR);
> +
> + hooks_ent = args_has(args, 'g') ? &global_hooks : &s->hooks;
> +
> + RB_FOREACH(hook, hooks, hooks_ent) {
> + used = xsnprintf(tmp, sizeof tmp, "%s -> ", hook->name);
> + cmd_list_print(hook->cmdlist, tmp + used, (sizeof tmp) - used);
> + cmdq_print(cmdq, "%s", tmp);
> + }
> + return (CMD_RETURN_NORMAL);
> +}
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
tmux-users mailing list
tmux-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tmux-users
