initial work towards moving away from Event.Trailing (closes #5)

[lrstanley]

Closes: https://github.com/lrstanley/girc/issues/35 Closes: https://github.com/lrstanley/girc/issues/19 See also: https://github.com/lrstanley/girc/pull/15#issuecomment-413845482

---

From: https://modern.ircdocs.horse/

Here are some examples of messages and how the parameters would be represented as JSON lists:

  :irc.example.com CAP * LIST :         ->  ["*", "LIST", ""]

  CAP * LS :multi-prefix sasl           ->  ["*", "LS", "multi-prefix sasl"]

  CAP REQ :sasl message-tags foo        ->  ["REQ", "sasl message-tags foo"]

  :dan!d@localhost PRIVMSG #chan :Hey!  ->  ["#chan", "Hey!"]

  :dan!d@localhost PRIVMSG #chan Hey!   ->  ["#chan", "Hey!"]

As the last two examples show, a trailing parameter (a parameter prefixed with ':') is another regular parameter. Once the ':' is stripped, software MUST just treat it as another param.

---

Essentially, Event.Trailing is now just an additional entry in the Event.Params slice.

I'm still opting to have the concept of Trailing as a semantic wrapper (and has nothing to do with the rfc's use of "trailing"). Event.Trailing(), which now just returns the last parameter/argument (from Event.Params directly), which will help as it is much more frequent to get the last element, than any other element (and Event.Params[len(Event.Params)-1] is very verbose). The other benefit is it defaults to an empty string. Maybe I'll rename it to Last?

This probably has bugs in it. Tests run fine, did some initial connections to servers, but I'd like some contributors to test this out before I merge into master:

• @nmeum
• @42wim
• @puffrfish
• @bmeh
• @Bevinsky
• @Kethsar

Also, @qaisjp, this has that spelling correction.

[codecov-io]

Codecov Report

Merging #36 into master will increase coverage by 0.13%. The diff coverage is 40%.

@@            Coverage Diff             @@
##           master      #36      +/-   ##
==========================================
+ Coverage   56.04%   56.18%   +0.13%     
==========================================
  Files          13       13              
  Lines        2339     2321      -18     
==========================================
- Hits         1311     1304       -7     
+ Misses        921      912       -9     
+ Partials      107      105       -2

Impacted Files	Coverage Δ
format.go	97.16% <ø> (ø)	:arrow_up:
commands.go	0% <0%> (ø)	:arrow_up:
cap.go	22.77% <0%> (ø)	:arrow_up:
cap_sasl.go	0% <0%> (ø)	:arrow_up:
client.go	70.27% <0%> (ø)	:arrow_up:
ctcp.go	69.49% <100%> (ø)	:arrow_up:
event.go	69.43% <39.53%> (+2.16%)	:arrow_up:
builtin.go	60.67% <53.57%> (-0.14%)	:arrow_down:
conn.go	58.53% <83.33%> (-0.82%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c1e59a0...057fbbb. Read the comment docs.

[nmeum]

First of all: Thanks for this patch, this is really useful to me.

I did some quick tests and the first thing I noticed is that /me messages are currently formated incorrectly bei Event.Pretty(). The code for formating them in event.go looks as follows:

if ctcp.Command == CTCP_ACTION {
    return fmt.Sprintf("[%s] **%s** %s", strings.Join(e.Params, ","), ctcp.Source.Name, ctcp.Text), true
}

and should prorably be changed to:

if ctcp.Command == CTCP_ACTION {
    return fmt.Sprintf("[%s] **%s** %s", strings.Join(e.Params[0:len(e.Params)-1], ","), ctcp.Source.Name, ctcp.Text), true
}

Otherwise they are formated as [#channel,ACTION foo] **user** foo instead of [#channel] **user** foo.

Might also be nice to add a helper method for getting all parameters except the trailing one (e.g. e.Params[0:len(e.Params)-1]) since this idiom is used in quite a few places now, but that's up to you.

[lrstanley]

@nmeum -- Latest commit should have fixes for that and TOPIC's.

I did decide to rename Event.Trailing() to Event.Last(), as Trailing() may mislead (since it's just the last param, not the trailing param necessarily).

I am not sure I want to add a helper to grab all but the last, because it's both not used too frequently (I think this is the only occurrence actually), but it's also not beneficial without also keeping track of if the last item was a trailing item (which means adding a field in Event that says it had a trailing argument).

[lrstanley]

@42wim have you had a chance to test this?

[42wim]

@lrstanley sorry, this issue was off my radar :( I've just tested the patch with matterbridge and it didn't found any issues yet. 👍