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 tab 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 tab less 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 Telegram 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 most replace the
current action pull request in the pull requestes/ section.

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