[3441] in BarnOwl Developers
Re: [barnowl] IRC reconnect (#108)
daemon@ATHENA.MIT.EDU (Jason Gross)
Mon May 27 13:59:23 2013
Date: Mon, 27 May 2013 10:59:13 -0700
From: Jason Gross <notifications@github.com>
Reply-To: barnowl/barnowl <reply+i-9709132-2001beb80c08e30b034e487c72b5d6a5a38438fb-4475081@reply.github.com>
To: barnowl/barnowl <barnowl@noreply.github.com>
In-Reply-To: <barnowl/barnowl/pull/108@github.com>
----==_mimepart_51a39ef1bd34f_5f6270bde068974
Date: Mon, 27 May 2013 10:59:13 -0700
Mime-Version: 1.0
Content-Type: text/plain;
charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Content-ID: <51a39ef1bf1cd_5f6270bde06906b@worker3.rs.github.com.mail>
Github drops line comments and commit comments on updates to a pull reque=
st, which is annoying. I'll respond here, so the comments stick around f=
or posterity.
@andersk commented:
> Why is .irc.channels a hidden file (in an already hidden directory)?
I think I misinterpreted geofft's zephyr:
> Class: barnowl Instance: irc-reconnect
Time: Sat Jan 5 14:16:33 2013 Host: TEAM-ROCKET.MIT.EDU
From: he now praises the iPad. <geofft>
>
> The proposal I made yesterday was no, and that you don't need to
bother maintaining any state other than .owl/ircchannels (or whatever
you decide to name the file. .irc.subs??), for code simplicity reasons
I'll rename it to `.owl/ircchannels` in the next update, unless people ha=
ve strong opinions.
> andersk commented on bb22c47 perl/modules/IRC/lib/BarnOwl/Module/IRC.pm=
:L378 10 hours ago
> `//` is Perl 5.10, which I don=E2=80=99t think we=E2=80=99ve started re=
quiring yet.
Oops. I'll change `my $filename =3D shift // (BarnOwl::get_config_dir() =
. "/$IRC_SUBS_FILENAME");` to
```perl
my $filename =3D shift;
$filename =3D File::Spec->catfile(BarnOwl::get_config_dir(), "$IRC_SUBS_F=
ILENAME") unless defined $filename;
```
> andersk commented on bb22c47 perl/modules/IRC/lib/BarnOwl/Module/IRC.pm=
:L359 10 hours ago
> You mean \$HOME/.owl/$IRC_SUBS_FILENAME.
Yes, I do, I'll fix that in the next update.
> andersk commented on 4121453 9 hours ago
> This adds a lot of binding complexity for something that=E2=80=99s prob=
ably easier to just reimplement in Perl.
I think this was going to eventually turn into a "shove all the I/O in Ba=
rnOwl onto a separate thread" branch, and, AIUI, perl doesn't currently s=
upport threading in any useful way. (Note: I haven't tested this, I'm ju=
st citing conversations I've read between davidben and others on -c barno=
wl.) However, I've set this branch aside for now, so I can go shove this=
commit into that branch for when/if I eventually do this, and reimplemen=
t this in perl, unless this is sufficient reason for the extra binding co=
mplexity. (I'd appreciate a definitive yes/no on this from @andersk or o=
thers, even though my default assumption will be "no, it's not sufficient=
reason".)
> andersk commented on bb22c47 perl/modules/IRC/lib/BarnOwl/Module/IRC.pm=
:L390 9 hours ago
> (Why C-side?)
(This on a comment "# Load the subs from the file; # TODO(jgross): Write =
a C-side function to do this, asynchronously") Same reason as above; AIU=
I, perl doesn't do async I/O in a useful way.
> andersk commented on bb22c47 perl/modules/IRC/lib/BarnOwl/Module/IRC.pm=
:L391 9 hours ago
> It=E2=80=99s safer to write this as `open(my $subsfile, '<', $filename)=
;`.
Ok, I'll fix this in the next update.
> andersk commented on bb22c47 perl/modules/IRC/lib/BarnOwl/Module/IRC.pm=
:L401 9 hours ago
> Why is that even supported, then?
>
> (In general, this seems like a silly file format. alias #channel would =
make more sense than #channel -a alias, would be more analogous to .zephy=
r.subs, and wouldn=E2=80=99t imply that -a alias is optional.)
That's a good question. Let me see if I can figure out why I did that.
---
Reply to this email directly or view it on GitHub:
https://github.com/barnowl/barnowl/pull/108#issuecomment-18509173=
----==_mimepart_51a39ef1bd34f_5f6270bde068974
Date: Mon, 27 May 2013 10:59:13 -0700
Mime-Version: 1.0
Content-Type: text/html;
charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Content-ID: <51a39ef1bfc89_5f6270bde06918@worker3.rs.github.com.mail>
<p>Github drops line comments and commit comments on updates to a pull re=
quest, which is annoying. I'll respond here, so the comments stick aroun=
d for posterity.</p>
<p><a href=3D"https://github.com/andersk" class=3D"user-mention">@andersk=
</a> commented:</p>
<blockquote>
<p>Why is .irc.channels a hidden file (in an already hidden directory)?</=
p>
</blockquote>
<p>I think I misinterpreted geofft's zephyr:</p>
<blockquote>
<p>Class: barnowl Instance: irc-reconnect<br>
Time: Sat Jan 5 14:16:33 2013 Host: TEAM-ROCKET.MIT.EDU<br>
From: he now praises the iPad. </p>
<p>The proposal I made yesterday was no, and that you don't need to<br>
bother maintaining any state other than .owl/ircchannels (or whatever<br>=
you decide to name the file. .irc.subs??), for code simplicity reasons</p=
>
</blockquote>
<p>I'll rename it to <code>.owl/ircchannels</code> in the next update, un=
less people have strong opinions.</p>
<blockquote>
<p>andersk commented on <a href=3D"https://github.com/barnowl/barnowl/com=
mit/bb22c47" class=3D"commit-link"><tt>bb22c47</tt></a> perl/modules/IRC/=
lib/BarnOwl/Module/IRC.pm:L378 10 hours ago<br><code>//</code> is Perl 5.=
10, which I don=E2=80=99t think we=E2=80=99ve started requiring yet.</p>
</blockquote>
<p>Oops. I'll change <code>my $filename =3D shift // (BarnOwl::get_confi=
g_dir() . "/$IRC_SUBS_FILENAME");</code> to</p>
<div class=3D"highlight"><pre><span class=3D"k">my</span> <span class=3D"=
nv">$filename</span> <span class=3D"o">=3D</span> <span class=3D"nb">shif=
t</span><span class=3D"p">;</span>
<span class=3D"nv">$filename</span> <span class=3D"o">=3D</span> <span cl=
ass=3D"nn">File::</span><span class=3D"n">Spec</span><span class=3D"o">-&=
gt;</span><span class=3D"n">catfile</span><span class=3D"p">(</span><span=
class=3D"nn">BarnOwl::</span><span class=3D"n">get_config_dir</span><spa=
n class=3D"p">(),</span> <span class=3D"s">"$IRC_SUBS_FILENAME"</span><sp=
an class=3D"p">)</span> <span class=3D"k">unless</span> <span class=3D"nb=
">defined</span> <span class=3D"nv">$filename</span><span class=3D"p">;</=
span>
</pre></div>
<blockquote>
<p>andersk commented on <a href=3D"https://github.com/barnowl/barnowl/com=
mit/bb22c47" class=3D"commit-link"><tt>bb22c47</tt></a> perl/modules/IRC/=
lib/BarnOwl/Module/IRC.pm:L359 10 hours ago<br>
You mean \$HOME/.owl/$IRC_SUBS_FILENAME.</p>
</blockquote>
<p>Yes, I do, I'll fix that in the next update.</p>
<blockquote>
<p>andersk commented on <a href=3D"https://github.com/barnowl/barnowl/com=
mit/4121453" class=3D"commit-link"><tt>4121453</tt></a> 9 hours ago<br>
This adds a lot of binding complexity for something that=E2=80=99s probab=
ly easier to just reimplement in Perl.</p>
</blockquote>
<p>I think this was going to eventually turn into a "shove all the I/O in=
BarnOwl onto a separate thread" branch, and, AIUI, perl doesn't currentl=
y support threading in any useful way. (Note: I haven't tested this, I'm=
just citing conversations I've read between davidben and others on -c ba=
rnowl.) However, I've set this branch aside for now, so I can go shove t=
his commit into that branch for when/if I eventually do this, and reimple=
ment this in perl, unless this is sufficient reason for the extra binding=
complexity. (I'd appreciate a definitive yes/no on this from <a href=3D=
"https://github.com/andersk" class=3D"user-mention">@andersk</a> or other=
s, even though my default assumption will be "no, it's not sufficient rea=
son".)</p>
<blockquote>
<p>andersk commented on <a href=3D"https://github.com/barnowl/barnowl/com=
mit/bb22c47" class=3D"commit-link"><tt>bb22c47</tt></a> perl/modules/IRC/=
lib/BarnOwl/Module/IRC.pm:L390 9 hours ago<br>
(Why C-side?)</p>
</blockquote>
<p>(This on a comment "# Load the subs from the file; # TODO(jgross): Wri=
te a C-side function to do this, asynchronously") Same reason as above; =
AIUI, perl doesn't do async I/O in a useful way.</p>
<blockquote>
<p>andersk commented on <a href=3D"https://github.com/barnowl/barnowl/com=
mit/bb22c47" class=3D"commit-link"><tt>bb22c47</tt></a> perl/modules/IRC/=
lib/BarnOwl/Module/IRC.pm:L391 9 hours ago<br>
It=E2=80=99s safer to write this as <code>open(my $subsfile, '<', $fil=
ename);</code>.</p>
</blockquote>
<p>Ok, I'll fix this in the next update.</p>
<blockquote>
<p>andersk commented on <a href=3D"https://github.com/barnowl/barnowl/com=
mit/bb22c47" class=3D"commit-link"><tt>bb22c47</tt></a> perl/modules/IRC/=
lib/BarnOwl/Module/IRC.pm:L401 9 hours ago<br>
Why is that even supported, then?</p>
<p>(In general, this seems like a silly file format. alias #channel would=
make more sense than #channel -a alias, would be more analogous to .zeph=
yr.subs, and wouldn=E2=80=99t imply that -a alias is optional.)</p>
</blockquote>
<p>That's a good question. Let me see if I can figure out why I did that=
.</p>
<p style=3D"font-size:small;-webkit-text-size-adjust:none;color:#666;">&m=
dash;<br>Reply to this email directly or <a href=3D'https://github.com/ba=
rnowl/barnowl/pull/108#issuecomment-18509173'>view it on GitHub</a>.<img =
src=3D'https://github.com/notifications/beacon/JJk3yKd0u6qAmPAmJXdf9_gRuU=
TQCipO-dP0x97pmbFICXxk9_eKsAFlYwLIF2Vi.gif' height=3D'1' width=3D'1'></p>=
----==_mimepart_51a39ef1bd34f_5f6270bde068974--
