Conversation
|
Exciting! Can't wait! :) |
host.go
Outdated
| user.SetHighlight(user.Name()) | ||
| } | ||
|
|
||
| // Gross hack to override line echo in golang.org/x/crypto/ssh/terminal |
There was a problem hiding this comment.
It's not quite clear, is this "dirty hack" in a code path even without the '/timestamps' feature being requested?
There was a problem hiding this comment.
@Osndok Correct, we fundamentally change how we render your own messages regardless of timestamps. This makes timestamps trivial (and possibly other things in the future).
|
|
This reverts commit 6ce14bf. We're going to do another approach.
chat/message/theme.go
Outdated
| // Timestamp formats and colorizes the timestamp. | ||
| func (theme Theme) Timestamp(t time.Time) string { | ||
| // TODO: Change this per-theme? Or config? | ||
| return theme.sys.Format(t.Format("2006-01-02 15:04:05 UTC")) |
There was a problem hiding this comment.
I don't particularly like the nearly-static yyyy-mm-dd & truly-static " UTC" strings on every line, but I find the reasoning for their inclusion to be adequate.
Although, I wonder if the reasoning would still hold after the user could explicitly set a timezone, or it were guessed by the incoming IP address... as the user would 'know' the time zone, and not need reminding every line.
I don't know how expensive the time.Format function is, but calling it for every timestamp trades runtime performance for code clarity.
There was a problem hiding this comment.
GeoIP and things like that is a lot more work, so really it's a question whether you think this should be merged now or if it should be blocked until someone someday writes the code to automatically detect timezones. :)
time.Format is not expensive enough to worry, ~everything else we do is much more expensive.
There was a problem hiding this comment.
You can use the TZ environment variable, if that being passed from a SSH client, like:
ssh -o SendEnv=TZ ...
4 participants
I have mixed feelings about this. It's not great, could be improved. Not sure if I'll get a chance to improve it anytime soon though.
Definitely no timezone support, or even per-second resolution. If this opens up those cans of worms, I'm going to be sad.