Edgewall Software
Modify

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#11342 closed defect (fixed)

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

Reported by: Ryan J Ollos Owned by: Jun Omae
Priority: normal Milestone: 1.0.2
Component: admin/console Version: 1.0-stable
Severity: normal Keywords: milestone date format
Cc: Branch:
Release Notes:

Help messages from trac-admin commands use the proper locale in the dateformat hint, and include a hint about using IS8601 format.

API Changes:

New function trac.admin.api:get_console_locale returns negotiated locale for the console.

Internal Changes:

Description (last modified by 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].

Attachments (0)

Change History (21)

comment:1 by Ryan J Ollos, 10 years ago

Description: modified (diff)
Status: newassigned

comment:2 by Ryan J Ollos, 10 years ago

Description: modified (diff)
Summary: Invalid date format for error message from trac-admin milestone complete operationInconsisten date format for error message and help message from trac-admin milestone complete operation

comment:3 by Ryan J Ollos, 10 years ago

Summary: Inconsisten date format for error message and help message from trac-admin milestone complete operationInconsistent date format for error message and help message from trac-admin milestone complete operation

comment:4 by Ryan J Ollos, 10 years ago

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?

comment:5 by Jun Omae, 10 years ago

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.

comment:6 by Ryan J Ollos, 10 years ago

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.

in reply to:  6 comment:7 by Jun Omae, 10 years ago

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 ("").

comment:8 by Ryan J Ollos, 10 years ago

Owner: changed from Ryan J Ollos to Jun Omae

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.

in reply to:  8 comment:9 by Jun Omae, 10 years ago

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 ("").

comment:10 by Ryan J Ollos, 10 years ago

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

    a b  
    1 $ LANG=en_US trac-admin tracdev milestone due
     1$ LANG=en_US.UTF-8 trac-admin tracdev milestone due
    22Error: Invalid arguments
    33
    44milestone due <name> <due>
     
    66    Set milestone due date
    77
    88    The <due> date must be specified in the "MMM d, yyyy h:mm:ss a" or "YYYY-
    9     MM-DDThh:mm:ss?hh:mm" (ISO 8601) format. Alternatively, "now" can be used
     9    MM-DDThh:mm:ss±hh:mm" (ISO 8601) format. Alternatively, "now" can be used
    1010    to set the due date to the current time. To remove the due date from a
    1111    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 ("").

in reply to:  8 ; comment:11 by Ryan J Ollos, 10 years ago

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.

in reply to:  10 ; comment:12 by Jun Omae, 10 years ago

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.

in reply to:  11 comment:13 by Jun Omae, 10 years ago

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.

in reply to:  12 comment:14 by Ryan J Ollos, 10 years ago

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

    diff --git a/trac/util/text.py b/trac/util/text.py
    index bd1f65a..bcd1193 100644
    a b def console_print(out, *args, **kwargs):  
    273273                   (defaults to `True`)
    274274    """
    275275    cons_charset = stream_encoding(out)
    276     out.write(' '.join([to_unicode(a).encode(cons_charset, 'replace')
    277                         for a in args]))
     276    try:
     277        out.write(' '.join([to_unicode(a).encode(cons_charset)
     278                            for a in args]))
     279    except UnicodeEncodeError:
     280        args = [a.replace(u'±', '+') for a in args]
     281        out.write(' '.join([to_unicode(a).encode(cons_charset, 'replace')
     282                            for a in args]))
    278283    if kwargs.get('newline', True):
    279284        out.write('\n')

comment:15 by Jun Omae, 10 years ago

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

    diff --git a/trac/util/text.py b/trac/util/text.py
    index bd1f65a..8eb7c03 100644
    a b def console_print(out, *args, **kwargs):  
    272272    :param kwargs: ``newline`` controls whether a newline will be appended
    273273                   (defaults to `True`)
    274274    """
    275     cons_charset = stream_encoding(out)
     275    from encodings.aliases import aliases
     276    cons_charset = stream_encoding(out).lower()
     277    if aliases.get(cons_charset.replace('-', '_'), cons_charset) != 'ascii':
     278        cons_charset = 'utf-8'
    276279    out.write(' '.join([to_unicode(a).encode(cons_charset, 'replace')
    277280                        for a in args]))
    278281    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.

comment:16 by Ryan J Ollos, 10 years ago

API Changes: modified (diff)
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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.

comment:17 by Jun Omae, 10 years ago

Resolution: fixed
Status: closedreopened

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)

comment:18 by Jun Omae, 10 years ago

Resolution: fixed
Status: reopenedclosed

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

comment:19 by Jun Omae, 10 years ago

Resolution: fixed
Status: closedreopened

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

in reply to:  19 comment:20 by Jun Omae, 10 years ago

Resolution: fixed
Status: reopenedclosed

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.

comment:21 by Ryan J Ollos, 10 years ago

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.