Inconsistent date format for error message and help message from trac-admin milestone complete operation

[Ryan J Ollos]

$ trac-admin tracdev milestone completed milestone110 10/10/2013
TracError: "10/10/2013" is an invalid date, or the date format is not known. Try "MM/DD/YY hh:mm:ss" instead

$ trac-admin tracdev milestone completed milestone110 10/10/2013 12:12:12
Error: Invalid arguments

milestone completed <name> <completed>

    Set milestone complete date

    The <completed> date must be specified in the "YYYY-MM-DD" format.
    Alternatively, "now" can be used to set the completion date to the current
    time. To remove the completion date from a milestone, specify an empty
    string ("").

I'm immediately reminded of [12087].

[Ryan J Ollos]

I see now that the reason for the help message in the second trac-admin call was that I didn't enclose the datetime in quotes.

The proposed change in log:rjollos.git:t11342 replaces uses of console_date_format_hint with get_datetime_format_hint(). The trac-admin commands call parse_date(text, format='datetime'), which will call get_datetime_format_hint if the date can't be parsed, so I think it makes sense to use the same for the help text.

Since the variable console_date_format_hint is now unused and is only left in place in case plugins are using it, it could be marked as deprecated. Should we mark it as deprecated, or just leave it?

[Jun Omae]

I don't think so. I think trac-admin command doesn't depend locale, shows and parses date time with YYYY-MM-DD or YYYY-MM-DD hh:mm:ss.

Trac [/var/trac/0.13]> milestone list

Name              Due         Completed
-------------------------------------------------
milestone1        2012-09-28  2013-12-31 00:00:00

Trac [/var/trac/0.13]> milestone completed milestone1 "2013-12-23 12:23:45"
Trac [/var/trac/0.13]> milestone list

Name              Due         Completed
-------------------------------------------------
milestone1        2012-09-28  2013-12-23 12:23:45

My suggestion is using hint=console_date_format_hint or hint='iso8601' for parse_date, instead of hint='datetime', see log:jomae.git:t11342.2.

[Ryan J Ollos]

I would prefer if we had the time in the hint as well since that is more consistent with the web interface, and fully describes the allowed value. If you want to use the string from trac.admin.api, perhaps we can add a console_datetime_format_hint: console_datetime_format_hint = 'YYYY-MM-DD HH:MM:SS', or console_date_format_hint = 'YYYY-MM-DD [HH:MM:SS]' if the latter is not too confusing and clearly indicates that the time is optional.

[Jun Omae]

I would prefer if we had the time in the hint as well since that is more consistent with the web interface, and fully describes the allowed value. If you want to use the string from trac.admin.api, perhaps we can add a console_datetime_format_hint: console_datetime_format_hint = 'YYYY-MM-DD HH:MM:SS', or console_date_format_hint = 'YYYY-MM-DD [HH:MM:SS]' if the latter is not too confusing and clearly indicates that the time is optional.

That makes sense. I like console_date_format_hint = 'YYYY-MM-DD [HH:MM:SS]'.

Another suggestion is specifying locale=... parameter of parse_date() while calling trac-admin, see log:jomae.git:t11342.3. After the changes, parse_date() accepts iso8601 string and localized date string, shows localized hint. But datetime is always formatted YYYY-MM-DD or YYYY-MM-DD hh:mm:ss. This is appropriate for calling from shell script.

LANG=en_US.UTF8

$ LANG=en_US.UTF8 make python=25 start-admin env=/var/trac/0.13
Welcome to trac-admin 1.0.2dev
Interactive Trac administration console.
Copyright (C) 2003-2013 Edgewall Software

Type:  '?' or 'help' for help on commands.

Trac [/var/trac/0.13]> help milestone due
milestone due <name> <due>

    Set milestone due date

    The <due> date must be specified in the "MMM d, yyyy h:mm:ss a" format.
    Alternatively, "now" can be used to set the due date to the current time.
    To remove the due date from a milestone, specify an empty string ("").

Trac [/var/trac/0.13]> milestone due milestone1 'Dec 21, 2013'
Trac [/var/trac/0.13]> milestone list

Name              Due         Completed
-------------------------------------------------
milestone1        2013-12-21  2013-12-23 12:23:45

LANG=de_DE.UTF8

$ LANG=de_DE.UTF8 make python=25 start-admin env=/var/trac/0.13 adminopts='help milestone due'
milestone due <name> <due>

    Set milestone due date

    The <due> date must be specified in the "dd.MM.yyyy HH:mm:ss" format.
    Alternatively, "now" can be used to set the due date to the current time.
    To remove the due date from a milestone, specify an empty string ("").

has no Babel

$ make python=25 start-admin env=/var/trac/0.13 adminopts='help milestone due'
milestone due <name> <due>

    Set milestone due date

    The <due> date must be specified in the "MM/DD/YY hh:mm:ss" format.
    Alternatively, "now" can be used to set the due date to the current time.
    To remove the due date from a milestone, specify an empty string ("").

[Ryan J Ollos]

I tried it out. It works great.

I have a small suggestion. Since ISO8601 format is allowed, we could modify the help string to say: The <due> date must be specified in the "MMM d, yyyy ​h:mm:ss a" or "YYYY-MM-DDThh:mm:ss±hh:mm" (ISO 8601) format. I was working on a patch for that the other day, and will try to finish it tomorrow.

[Jun Omae]

I have a small suggestion. Since ISO8601 format is allowed, we could modify the help string to say: The <due> date must be specified in the "MMM d, yyyy ​h:mm:ss a" or "YYYY-MM-DDThh:mm:ss±hh:mm" (ISO 8601) format.

That makes sense. I've worked on log:jomae.git:t11342.4 rebased with 1.0-stable.

$ make python=25 start-admin env=/var/trac/0.13 adminopts='help milestone due'
milestone due <name> <due>

    Set milestone due date

    The <due> date must be specified in the "MMM d, yyyy h:mm:ss a" or "YYYY-
    MM-DDThh:mm:ss±hh:mm" (ISO 8601) format. Alternatively, "now" can be used
    to set the due date to the current time. To remove the due date from a
    milestone, specify an empty string ("").

[Ryan J Ollos]

When the shell environment is not unicode, the ± is replaced by ?. I suppose nothing can be done about this, but it is just unfortunate that a user might be confused by the ? if they don't have a unicode shell.

(a) out1 vs. (b) out2

@@ -1 @@
-$ LANG=en_US trac-admin tracdev milestone due
+$ LANG=en_US.UTF-8 trac-admin tracdev milestone due
 Error: Invalid arguments
 
 milestone due <name> <due>
@@ -6 +6 @@
     Set milestone due date
 
     The <due> date must be specified in the "MMM d, yyyy h:mm:ss a" or "YYYY-
-    MM-DDThh:mm:ss?hh:mm" (ISO 8601) format. Alternatively, "now" can be used
+    MM-DDThh:mm:ss±hh:mm" (ISO 8601) format. Alternatively, "now" can be used
     to set the due date to the current time. To remove the due date from a
     milestone, specify an empty string ("").

I also found that even when specifying UTF-8 for the environment, if the language pack is not installed, the non-ascii characters are replaced with ?.

$ LANG=de_DE.UTF-8 trac-admin tracdev milestone due
Fehler: Ung?ltige Argumente

milestone due <name> <due>

    Set milestone due date

    The <due> date must be specified in the "dd.MM.yyyy HH:mm:ss" or "YYYY-MM-
    DDThh:mm:ss?hh:mm" (ISO 8601) format. Alternatively, "now" can be used to
    set the due date to the current time. To remove the due date from a
    milestone, specify an empty string ("").

$ sudo aptitude install language-pack-de
[...]
Generating locales...
  de_AT.UTF-8... done
  de_BE.UTF-8... done
  de_CH.UTF-8... done
  de_DE.UTF-8... done
  de_LI.UTF-8... done
  de_LU.UTF-8... done
Generation complete.
Processing triggers for bamfdaemon ...
Rebuilding /usr/share/applications/bamf-2.index...

$ LANG=de_DE.UTF-8 trac-admin tracdev milestone due
Fehler: Ungültige Argumente

milestone due <name> <due>

    Set milestone due date

    The <due> date must be specified in the "dd.MM.yyyy HH:mm:ss" or "YYYY-MM-
    DDThh:mm:ss±hh:mm" (ISO 8601) format. Alternatively, "now" can be used to
    set the due date to the current time. To remove the due date from a
    milestone, specify an empty string ("").

[Ryan J Ollos]

Replying to rjollos:

I have a small suggestion. Since ISO8601 format is allowed, we could modify the help string to say: The <due> date must be specified in the "MMM d, yyyy ​h:mm:ss a" or "YYYY-MM-DDThh:mm:ss±hh:mm" (ISO 8601) format. I was working on a patch for that the other day, and will try to finish it tomorrow.

I added two small changes. I'm less confident of the second, but since ISO8601 format is always parsed, I was thinking it might be a good idea to specify it in the hint: log:rjollos.git:t11342.4. Let me know if you think it is okay.

[Jun Omae]

When the shell environment is not unicode, the ± is replaced by ?. I suppose nothing can be done about this, but it is just unfortunate that a user might be confused by the ? if they don't have a unicode shell.

The help text is displayed by printout. The method converts with sys.stdout.encoding. Also, the encoding depends on LC_CTYPE environment on Unix. The encoding depends on ANSI codepage on Windows. However, if stdout is not a tty, it would be None. In this case, printout would use utf-8 encoding. See console_print in trac/util/text.py.

Linux:

$ LC_CTYPE=en_US.UTF8 python2.5 -c 'import sys; print sys.stdout.encoding'
UTF-8
$ LC_CTYPE=ja_JP.EUC-JP python2.5 -c 'import sys; print sys.stdout.encoding'
EUC-JP
$ LC_CTYPE=POSIX python2.5 -c 'import sys; print sys.stdout.encoding'
ANSI_X3.4-1968
$ LC_CTYPE=en_US.UTF8 python2.5 -c 'import sys; print sys.stdout.encoding' | cat
None

Windows for Japanese Edition:

C> python25 -c "import sys; print sys.stdout.encoding"
cp932

C> python25 -c "import sys; print sys.stdout.encoding" | more
None

Should we use only ASCII characters for ISO 8601 hint? e.g. YYYY-MM-DDThh:mm:ss+hh:mm or YYYY-MM-DDThh:mm:ss[+-]hh:mm.

[Jun Omae]

I added two small changes. I'm less confident of the second, but since ISO8601 format is always parsed, I was thinking it might be a good idea to specify it in the hint: log:rjollos.git:t11342.4. Let me know if you think it is okay.

Okay. That's good.

[Ryan J Ollos]

Replying to jomae:

Should we use only ASCII characters for ISO 8601 hint? e.g. YYYY-MM-DDThh:mm:ss+hh:mm or YYYY-MM-DDThh:mm:ss[+-]hh:mm.

The YYYY-MM-DDThh:mm:ss+hh:mm would probably be the least confusing, using the + similar to how the dateformat hint sometimes uses a for am/pm.

If we think this might come up with reasonable frequency, then I guess it would be better to go for printing ASCII only. I'm not sure how often that would be encountered though, and if rarely, then ± is probably better since it is more readable.

I had some other ideas, but I'm not sure if they are good. One would be to print a warning when the terminal is ASCII. Another, which might be too ugly, would be to selectively replace characters:

trac/util/text.py

@@ -273 +273 @@
                    (defaults to `True`)
     """
     cons_charset = stream_encoding(out)
-    out.write(' '.join([to_unicode(a).encode(cons_charset, 'replace')
-                        for a in args]))
+    try:
+        out.write(' '.join([to_unicode(a).encode(cons_charset)
+                            for a in args]))
+    except UnicodeEncodeError:
+        args = [a.replace(u'±', '+') for a in args]
+        out.write(' '.join([to_unicode(a).encode(cons_charset, 'replace')
+                            for a in args]))
     if kwargs.get('newline', True):
         out.write('\n')

[Jun Omae]

Another idea, if the terminal encoding is ascii, would be to print as utf-8. I think ascii encoding rarely is used.

trac/util/text.py

@@ -272 +272 @@
     :param kwargs: ``newline`` controls whether a newline will be appended
                    (defaults to `True`)
     """
-    cons_charset = stream_encoding(out)
+    from encodings.aliases import aliases
+    cons_charset = stream_encoding(out).lower()
+    if aliases.get(cons_charset.replace('-', '_'), cons_charset) != 'ascii':
+        cons_charset = 'utf-8'
     out.write(' '.join([to_unicode(a).encode(cons_charset, 'replace')
                         for a in args]))
     if kwargs.get('newline', True):

But we should do nothing for this issue since we think it is rare issue. Please go ahead with log:rjollos.git:t11342.4.

[Ryan J Ollos]

Committed to 1.0-stable in [12277:12278], and merged to trunk in [12279].

I accidentally left a debug statement in [12277#file5], which was reverted in [12278#file3]. Sorry about that.

[Jun Omae]

The following failure was reported in comment:8:ticket:11366.

ERROR: test_get_console_locale (trac.admin.tests.console.TracadminTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\Trac\repos\1.0-stable\trac\admin\tests\console.py", line 188, in _test_get_console_locale_with_babel
    self.assertEqual(Locale.default(), get_console_locale(None, None))
  File "c:\trac\dependencies\babel-0.9.x\babel\core.py", line 155, in default
    return cls(default_locale(category, aliases=aliases))
  File "c:\trac\dependencies\babel-0.9.x\babel\core.py", line 137, in __init__
    raise UnknownLocaleError(identifier)
UnknownLocaleError: unknown locale ''

Reproduced if LANG environment is empty even on Linux.

$ LANG= PYTHONPATH=. ~/venv/py25/bin/python trac/admin/tests/console.py
...
FAILED (errors=1)

[Jun Omae]

Fixed in [12283] and merged in [12284].

[Jun Omae]

Oops. After r12277, [[TracAdminHelp(milestone due)]] and TracAdmin page lead Genshi UnicodeDecodeError.

[Jun Omae]

Replying to jomae:

Oops. After r12277, [[TracAdminHelp(milestone due)]] and TracAdmin page lead Genshi UnicodeDecodeError.

I just created new ticket for the issue, #11549. The root cause is another thing. This ticket is closing.

[Ryan J Ollos]

In [12876], minor revision to [12277]. Merged in [12877:12878] (sorry for the mess, I merged into a dirty working copy).