318422 - Timestamp format should be more flexible

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, enhancement)

Description

[Jeff]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Chatzilla 0.9.68.5.1 [Firefox 1.5/undefined]

The timestamp field does not have any sort of identifier for 12 hour format versus 24 hour format. The only identifier is a 24 hour format.

Reproducible: Always

Steps to Reproduce:




There is no identifier for 12 hour format at all.

Comment 1

[Jeff]

Changed my mind and made this a feature enhancement instead.

Comment 2

[Glen Mailer]

I have written a javascript implementation of strftime for this purpose (before the bug was posted).

It is scheduled to be included with the next release as far as i know.

Comment 3

[Glen Mailer]

Attachment: strftime implementation

this seems to have been lost somewhere along the line, so i'm attaching it.

Comment 4

[Rob Marshall [tH]]

Attachment: patch v3

Work in progress. This is Glen's strftime from above, plus modifications and code by James Ross to replace the CSS generated content timestamp system, plus my own code to integrate the two.

Currently log and display timestamps are separate, and both default to native formatting. I'm not sure about this, because we may want to be consistent across computers and promote date formats that everyone can read (y-m-d). There's no migration from the old prefs and I haven't thought much about what should be visible in the UI in what way.

Comments very much welcome. :)

Comment 5

[Rob Marshall [tH]]

Attachment: patch v4

I think this is ready for review. I've dumped native formatting and not coded migration because there are too many edge cases to work out and I assume people who customize timestamps won't mind doing it again just this once.

I haven't made big UI changes, just removed /timestamp-format and changed the pref help text. If anybody has other ideas, new bugs would probably be best.

Comment 6

[Samuel Sieb]

Comment on attachment 237538 [details] [diff] [review]
patch v4

>+        if (ary[2] in timestr) {
>+            var tbranch = timestr[ary[2]];

Coding style.  The "{" goes on the next line.  (You did it for most of the code, just missed a couple of places.)

>+            if ("pad" in tbranch) {
>+                var padwith = (padwith in tbranch) ? tbranch.padwith : "0";

Again.

>+        if (!rpl) {
>+            switch (ary[2])

And here.

Comment 7

[:Gijs (he/him)]

Checked in with nits fixed:

Checking in mozilla/extensions/irc/js/lib/utils.js;
/cvsroot/mozilla/extensions/irc/js/lib/utils.js,v  <--  utils.js
new revision: 1.66; previous revision: 1.65
done
Checking in mozilla/extensions/irc/xul/content/commands.js;
/cvsroot/mozilla/extensions/irc/xul/content/commands.js,v  <--  commands.js
new revision: 1.108; previous revision: 1.107
done
Checking in mozilla/extensions/irc/xul/content/prefs.js;
/cvsroot/mozilla/extensions/irc/xul/content/prefs.js,v  <--  prefs.js
new revision: 1.40; previous revision: 1.39
done
Checking in mozilla/extensions/irc/xul/content/static.js;
/cvsroot/mozilla/extensions/irc/xul/content/static.js,v  <--  static.js
new revision: 1.211; previous revision: 1.210
done
Checking in mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties;
/cvsroot/mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties,v  <--  chatzilla.properties
new revision: 1.119; previous revision: 1.118
done

Comment 8

[Robert Kaiser]

Comment on attachment 237538 [details] [diff] [review]
patch v4

>+pref.group.appearance.timestamps.help  = The Format preference uses strftime replacements. For example, "%l %H:%M:%S%P" might become "Thursday 1:37:42pm".

From what I see, this example is wrong. To get the date string you have given, the format is "%A %l:%M:%S%P" if I'm reading the strftime manual correctly...

Comment 9

[:Gijs (he/him)]

(In reply to comment #8)
> (From update of attachment 237538 [details] [diff] [review] [edit])
> >+pref.group.appearance.timestamps.help  = The Format preference uses strftime replacements. For example, "%l %H:%M:%S%P" might become "Thursday 1:37:42pm".
> 
> From what I see, this example is wrong. To get the date string you have given,
> the format is "%A %l:%M:%S%P" if I'm reading the strftime manual correctly...

Looking at the code, I think he's right. Robert, can you confirm that and/or poke up a patch?

Comment 10

[Rob Marshall [tH]]

Attachment: [checked in] fix help text

Yep, exactly right. Thanks!

Comment 11

[:Gijs (he/him)]

Comment on attachment 244769 [details] [diff] [review]
[checked in] fix help text

Fixed:

Checking in mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties;
/cvsroot/mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties,v  <--  chatzilla.properties
new revision: 1.122; previous revision: 1.121
done