Re: [dev] [ii]: path to apply action command

From: Markus Teich <markus.teich_AT_stusta.mhn.de>
Date: Mon, 18 Sep 2017 14:35:50 +0200

Heyho,

Hiltjo Posthuma wrote:
> Minor nitpick, I'd prefer a space after the wildcard:
> > + snprintf(msg, sizeof(msg), "* %s %s", nick, &buf[3]);

I think that looks weird. No actual research, but I think I've seen the format
without a space more often and the `*` is not allowed in nick names, so no
source of confusion there either.

> > - snprintf(msg, sizeof(msg), "<%s> %s", argv[TOK_NICKSRV],
> > - argv[TOK_TEXT] ? argv[TOK_TEXT] : "");
> > + if (sscanf(argv[TOK_TEXT], "\01ACTION%[^\01]\01", text) == 1)
> > + snprintf(msg, sizeof(msg), "*%s %s", argv[TOK_NICKSRV],
> > + *text ? &text[1] : "");
>
> You should use a regular check here (strncmp?) without the extra text buffer.

if (strncmp(argv[TOK_TEXT], "\01ACTION", strlen("\01ACTION")) == 0 && argv[TOK_TEXT][strlen(argv[TOK_TEXT])-1] == '\01')

is not much better imho…

> With the changes I think its reasonable to include this in the upstream
> version. Any thoughts or is it bloat?

According to my limited IRC usage the /me actions are used frequently and thus
this is worhtwhile to add upstream. In the end it's up to you as the maintainer.
If you don't want to put it upstream, I'd like to see it at least replace the
current action patch in the patches/ section.

--Markus
Received on Mon Sep 18 2017 - 14:35:50 CEST