Date: Thu, 29 Sep 2022 12:45:23 +0000
To: delthas <delthas@dille.cc>
From: Simon Ser <contact@emersion.fr>
Cc: ~emersion/soju-dev@lists.sr.ht
Subject: Re: [PATCH] Fix clearing webpush targets after any MARKREAD
Message-ID: <ZI9f99gc4icLStcWYQd9KOvIAxrWYfYChfWdltgYHTd39LjLliTqlWbCcYValLjOxzjXhUi74ld-Zh4D6dhCF0Je3AQoLrvYwKKD7zu9PAg=@emersion.fr>
In-Reply-To: <20220928172350.16206-1-delthas@dille.cc>
References: <20220928172350.16206-1-delthas@dille.cc>
Feedback-ID: 1358184:user:proton
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Nice! A few comments below.

On Wednesday, September 28th, 2022 at 19:23, delthas <delthas@dille.cc> wro=
te:

> diff --git a/irc.go b/irc.go
> index bb17d64..dd56f36 100644
> --- a/irc.go
> +++ b/irc.go
> @@ -459,10 +459,24 @@ func (cm *monitorCasemapMap) ForEach(f func(name st=
ring, online bool)) {
>  =09}
>  }
> =20
> -type casemapSet struct{ casemapMap }
> +type pushTargetCasemapMap struct{ casemapMap }
> =20
> -func (cs *casemapSet) Add(name string) {
> -=09cs.set(name, nil)
> +func (cm *pushTargetCasemapMap) Get(name string) (last time.Time) {
> +=09if v :=3D cm.get(name); v =3D=3D nil {
> +=09=09return time.Time{}
> +=09} else {
> +=09=09return v.(time.Time)
> +=09}
> +}
> +
> +func (cm *pushTargetCasemapMap) Set(name string, last time.Time) {
> +=09cm.set(name, last)
> +}
> +
> +func (cm *pushTargetCasemapMap) ForEach(f func(name string, last time.Ti=
me)) {
> +=09for _, entry :=3D range cm.m {
> +=09=09f(entry.originalKey, entry.value.(time.Time))
> +=09}
>  }

Can we drop the ForEach impl if it's not used anywhere?

> =20
>  func isWordBoundary(r rune) bool {
> diff --git a/upstream.go b/upstream.go
> index e1030a2..180da98 100644
> --- a/upstream.go
> +++ b/upstream.go
> @@ -551,7 +551,9 @@ func (uc *upstreamConn) handleMessage(ctx context.Con=
text, msg *irc.Message) err
> =20
>  =09=09if highlight || uc.isOurNick(target) {
>  =09=09=09go uc.network.broadcastWebPush(msg)
> -=09=09=09uc.network.pushTargets.Add(bufferName)
> +=09=09=09if timestamp, err :=3D time.Parse(xirc.ServerTimeLayout, string=
(msg.Tags["time"])); err =3D=3D nil {
> +=09=09=09=09uc.network.pushTargets.Set(bufferName, timestamp)
> +=09=09=09}

Hm, what happens if upstream doesn't support server-time? We should probabl=
y
fall back to time.Time or something right?