Add human-readable idle time in whois info #721

astorije · 2016-10-27T05:29:26Z

One step towards #560.

See http://superuser.com/a/272069/208074 for explanation on /whois nick nick instead of /whois nick.

Results

This PR used to say "since X days, X hours, X minutes and X seconds" but was moved to the format above for simplicity and consistency with other date/times in the current UI.
Helper and tests to do that have been moved to the archive PR #819.

MaxLeiter · 2016-10-27T05:41:13Z

bit of a nitpick, but I think there should be an oxford comma after the minutes before the and.

astorije · 2016-10-27T05:50:20Z

I thought about it, then decided to not do that. I can update if people have strong feelings about that.

xPaw · 2016-10-27T10:14:35Z

client/views/actions/whois.tpl

+{{#if whois.idle}}
+<div>
+	<span role="button" class="user {{colorClass whois.nick}}" data-name="{{whois.nick}}">{{whois.nick}}</span>
+	has been idle for {{humanReadableRange whois.idle}}.


Tooltip with just the date.

xPaw · 2016-10-27T10:15:41Z

client/js/lounge.js

@@ -844,7 +844,7 @@ $(function() {

 		socket.emit("input", {
 			target: chat.data("id"),
-			text: "/whois " + name
+			text: "/whois " + name + " " + name


@DanielOaks mind saying if there could be any problems with this?

Hmm I'll take a look, do some testing and let you know

@DanielOaks, any luck? :)

@astorije Ah, sorry. I had a squiz, it's not really defined anywhere but servers out there process it without an issue. Some like Insp also seem to require it to send all the WHOIS info. Should be fine, yeah.

@astorije a comment in the code here, explaining why the name is repeated, might be helpful in the future

@xPaw, @DanielOaks, if you look at the third commit of this PR, I actually moved that logic to the server through a new input file.
Reason behind that is that with the second commit alone, clicking on a nick and typing /whois nick were giving different outputs, and it confused me for a second. Now whois nick directly sends /whois nick nick (just like /away sends /away <empty reason>) and it's nicer for the user.
I'll squash commits 2 and 3 if you're ok with that (commit 3 cancels out commit 2, but I kept it for now just in case).

@PolarizedIons, done.

AlMcKinlay

Mostly happy with this, but I agree about the oxford comma. It doesn't actually matter, but it'll annoy me every time I use the feature if you don't put it in :-P

astorije · 2016-12-19T03:33:45Z

@YaManicKill, @xPaw, I ended up moving away from the "since X days, X hours, ..." format I created and used the same date-time format we use for other dates in the UI. Reason is that when we start using relative dates for the date marker with moment, we can add it here as well and move the full date in a tooltip so it says "has been idle for about 2 hours" or the like.
It makes for a simpler PR and my helper is more likely to be buggy anyway.

For the oxford comma, I started to look at it then gave up as I would have had to make sure there is no comma with only 2 values. Painful. Better handled later.

xPaw · 2016-12-19T10:18:44Z

src/plugins/inputs/whois.js

+	// This queries server of the other user and not of the current user, which
+	// does not know idle time.
+	// See http://superuser.com/a/272069/208074.
+	network.irc.raw("WHOIS", args[0], args[0]);


Yeeah, this might have side effects. As the command is described as this: WHOIS [<server>] <nickmask>[,<nickmask>[,...]]

So if you tried to query nick on a server, your command would not work.

Good call. How do you suggest I do that? Solve it on the server or just cancel and go back to client-side only?

If server-side, what about:

if (args.length === 1) { network.irc.raw("WHOIS", args[0], args[0]); } else { network.irc.raw("WHOIS", args.join(" ")); }

or something like that?
/whois nick would always be /whois nick nick, and everything else would be forwarded as is.

args.join(" ") seems like it'd combine all args into the trailing, so would it end up sending out something like this?

WHOIS :arg1 arg2

That wouldn't go well. If that's how it'd send out with the above, you'd need to make it push all args separately instead (not sure how you'd do that in JS, but in other langs it'd look something like network.irc.raw("WHOIS", args...);

I guess that would be network.irc.raw("WHOIS", ...args); in JS with ES6's spread operator. Will wait for a LGTM or better suggestion from @xPaw.

Scratch that, I re-assembled the whole command that gets parsed in client.js and it works just fine (spread operator is not available in Node.js v4...).

xPaw · 2016-12-19T10:20:04Z

src/plugins/irc-events/whois.js

@@ -27,6 +27,9 @@ module.exports = function(irc, network) {
 				text: "No such nick: " + data.nick
 			});
 		} else {
+			// Absolute datetime in milliseconds since nick is idle
+			data.idleTime = Date.now() - data.idle * 1000;


Not entirely sure if whois messages can have server-time, but if they do, use that instead maybe?

I don't know much about server-time but if that's supposed to come along with data, then I don't receive it there.

Looking at irc-framework's handlers, I don't see such evidence. Can you confirm or help me a bit on this?

Hmm maybe that's just missing in the framework. cc @prawnsalad

I suggest going with what we have right now and improving if/when irc-framework adds this (if possible). In the meantime, this works just fine (and also I am sooo over that PR :D).

xPaw

Revert /whois command.

astorije · 2016-12-22T05:25:53Z

@xPaw, see my latest commit, I think that's the best compromise: WHOIS with a single argument gets repeated, everything else gets sent raw as-is.

This queries server of the other user and not current user, which does not know idle time. See http://superuser.com/a/272069/208074. Override is done before command is being sent to the server: if a single argument is given to `/whois`, it is being repeated, otherwise the command is sent as is.

Add human-readable idle time in whois info

astorije added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Oct 27, 2016

astorije force-pushed the astorije/whois-idle branch from 6656b9c to 61de123 Compare October 27, 2016 05:30

xPaw reviewed Oct 27, 2016

View reviewed changes

AlMcKinlay requested changes Dec 12, 2016

View reviewed changes

astorije force-pushed the astorije/whois-idle branch from 1960eed to 5c873c8 Compare December 19, 2016 03:17

astorije mentioned this pull request Dec 19, 2016

Seconds-to-human-readable-range Handlebars helper and tests #819

Closed

astorije added this to the 2.2.0 milestone Dec 19, 2016

AlMcKinlay approved these changes Dec 19, 2016

View reviewed changes

xPaw reviewed Dec 19, 2016

View reviewed changes

xPaw requested changes Dec 21, 2016

View reviewed changes

Add human-readable idle time in whois info

7ae11ba

astorije force-pushed the astorije/whois-idle branch from 5c873c8 to 48e5ba6 Compare December 22, 2016 05:22

xPaw approved these changes Dec 22, 2016

View reviewed changes

astorije force-pushed the astorije/whois-idle branch from 48e5ba6 to da2e286 Compare December 23, 2016 07:04

astorije merged commit c0fa58a into master Dec 23, 2016

astorije deleted the astorije/whois-idle branch December 23, 2016 08:42

PolarizedIons mentioned this pull request Jan 2, 2017

/whois on a person who is not online, results in a "No such server" error #864

Closed

matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017

Merge pull request thelounge#721 from thelounge/astorije/whois-idle

b88610d

Add human-readable idle time in whois info

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add human-readable idle time in whois info #721

Add human-readable idle time in whois info #721

astorije commented Oct 27, 2016 •

edited

Loading

MaxLeiter commented Oct 27, 2016

astorije commented Oct 27, 2016

xPaw Oct 27, 2016

xPaw Oct 27, 2016

DanielOaks Oct 27, 2016

astorije Dec 7, 2016

DanielOaks Dec 7, 2016 •

edited

Loading

PolarizedIons Dec 13, 2016

astorije Dec 19, 2016

AlMcKinlay left a comment

astorije commented Dec 19, 2016

xPaw Dec 19, 2016

astorije Dec 20, 2016

DanielOaks Dec 20, 2016

astorije Dec 20, 2016

astorije Dec 22, 2016

xPaw Dec 19, 2016

astorije Dec 20, 2016

xPaw Dec 21, 2016

astorije Dec 22, 2016

xPaw left a comment

astorije commented Dec 22, 2016

Add human-readable idle time in whois info #721

Add human-readable idle time in whois info #721

Conversation

astorije commented Oct 27, 2016 • edited Loading

Results

MaxLeiter commented Oct 27, 2016

astorije commented Oct 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielOaks Dec 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlMcKinlay left a comment

Choose a reason for hiding this comment

astorije commented Dec 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xPaw left a comment

Choose a reason for hiding this comment

astorije commented Dec 22, 2016

astorije commented Oct 27, 2016 •

edited

Loading

DanielOaks Dec 7, 2016 •

edited

Loading