Merge lp:~songofacandy/bzr/i18n-command into lp:bzr

Proposed by methane
Status: Merged
Merged at revision: 5994
Proposed branch: lp:~songofacandy/bzr/i18n-command
Merge into: lp:bzr
Diff against target: 571 lines (+272/-33)
8 files modified
bzrlib/builtins.py (+1/-0)
bzrlib/commands.py (+48/-25)
bzrlib/export_pot.py (+14/-5)
bzrlib/help.py (+2/-2)
bzrlib/help_topics/__init__.py (+1/-0)
bzrlib/i18n.py (+1/-1)
bzrlib/tests/test_export_pot.py (+46/-0)
bzrlib/tests/test_help.py (+159/-0)
To merge this branch: bzr merge lp:~songofacandy/bzr/i18n-command
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
Review via email: mp+62752@code.launchpad.net

Description of the change

Translate command help

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :
Download full text (4.6 KiB)

8 +

Spurious ?

20 + i18n.uninstall()
21 +

Hmm, that's weird, who did install what ? Why do we need to start
uninstalling ?

Said otherwise, this reveals some weird initialization and I
think we want a better way to achieve it.

To avoid a controversial discussion about general deployment I
think you should focus on initializing the i18n/i10n only for the
help command. We can revisit that later once we are happy with
the results.

46 + # NOTE: If cmd_gettext translates ':Usage:\n', the section will
47 + # be shown after "Description" section.
48 + doc = cmd_gettext(doc)

/me blinks

I'm sure this will be eaiser to understand if you added a
corresponding test.

58 - result += ':Purpose: %s\n' % purpose
59 + result += ':%s: %s\n' % (gettext('Purpose'), purpose)

Hmm, this pattern will probably occurs quite frequently and I'm
pretty sure the translators will sometimes need to tweak the
format Is there some existing facility we can reuse so this can
be spelt something like:

  result += gettext(':Purpose: %s\n' % (gettext(purpose),)

or even better:

  result += gettext_xxx(':Purpose: %(purpose)s\n', dict(purpose=purpose))

?

74 + options = option.get_optparser(self.options(), True)
75 + options = options.format_option_help()

How about:

 parser = option.get_optparser(self.options(), True)
 options = parser.format_option_help()

88 + if options.startswith('Options:') or options.startswith('options:'):
89 + # Python 2.4 version of optparse uses 'options'.
90 + result += ':%s:%s' % (gettext('Options'), options[len('options:'):])

Forget about python-2.4, we don't support it anymore.

135 - sys.stdout.write(self.get_help_text())
136 + self._setup_outf()
137 + self.outf.write(self.get_help_text())
138 return 0
139 if 'usage' in opts: # e.g. bzr add --usage
140 sys.stdout.write(self.get_help_text(verbose=False))

/me blinks

Nice to address this bug, but why on earth is it needed ? This
should really be addressed far earlier no ? Can you find where or
do you need help here ?

Also, it looks like the exact same bug affect 'usage'. I realize
this may be out of scope for this proposal, so may be we want a
different proposal that, just let me know if you want to do it or
leave it to me.

145 + @staticmethod
146 + def get_gettext():

I don't understand why you need a static method there ? I realize
*you* don't need 'self' but may be some plugins will, no ?

149 + NOTE: Commands provided by plugins should override this to use own
150 + i18n system.

typo/style nits: We don't use 'NOTE:' all caps, may be just 'Note:' or may be just nothing ;) s/to use own/to use their own/

165 + opt_no_i18n = opt_no_aliases = False

Excuse my ignorance, but shouldn't that be opt_no_i10n here ?

182 + if not opt_no_i18n:
183 + # selftest uninstalls i18n later.
184 + i18n_install()

Ha ! Here is the install !

So, yeah, it will be clearer to start with if install/uninstall
is done by the help command only and the appropriate tests,
keeping it under control will help us understand how/when/where
it's needed.

We had trouble in the past about such global options, I'd rather
avoid the fallbacks right from the start.

This is by no means a lack of trust, I'm just being caut...

Read more...

review: Needs Information
Revision history for this message
Vincent Ladeuil (vila) wrote :

Also ! This proposal is a significant step in the i10n process as it will lead to the first visible results for our users so it definitely deserves news entries (and credits !).

Don't worry about writing imperfect English, we'll find native speakers to catch *my* spelling mistakes :)

Revision history for this message
methane (songofacandy) wrote :

> 8 +
>
> Spurious ?

I've removed it.

>
> 20 + i18n.uninstall()
> 21 +
>
> Hmm, that's weird, who did install what ? Why do we need to start
> uninstalling ?

I think of many other commands like log or status use i18n in the future.
I18n only breaks selftest.

>
> Said otherwise, this reveals some weird initialization and I
> think we want a better way to achieve it.
>
> To avoid a controversial discussion about general deployment I
> think you should focus on initializing the i18n/i10n only for the
> help command. We can revisit that later once we are happy with
> the results.

I am not against on enabling i18n only for help command until other
commands needs i18n.

But, how? When selftest calls help, enabling i18n breaks many tests.
I don't think replacing `i18n.uninstall()` with global variable like
`opt_no_i18n=True` is beautiful answer.

One idea come to my mind is adding `use_i18n = False` class attribute
to the Command class. The cmd_help overrides it with `use_i18n = True`.
Then, selftest overrides `use_i18n = False` again.

Do you think OK with this idea?

Revision history for this message
Vincent Ladeuil (vila) wrote :

> I think of many other commands like log or status use i18n in the future.

I expect so !

> I18n only breaks selftest.

Let's focus on that then.

> I am not against on enabling i18n only for help command until other
> commands needs i18n.

Cool.

>
> But, how? When selftest calls help, enabling i18n breaks many tests.

I think we don't want to blindly activate i18n during tests. Quite the contrary, we probably want to *never* activate it by default (which should translate to either totally disabling any i18n code or may be just using a NullTranslation ?)

So, this kind of isolation is generally done in bzrlib.tests.TestCase, if you push a revision with some tests failing, I should be able to fix that.

Revision history for this message
methane (songofacandy) wrote :
Download full text (5.0 KiB)

> 46 + # NOTE: If cmd_gettext translates ':Usage:\n', the section will
> 47 + # be shown after "Description" section.
> 48 + doc = cmd_gettext(doc)
>
> /me blinks
>
> I'm sure this will be eaiser to understand if you added a
> corresponding test.

Is the test for garbage input and garbage output really needed?

I've modified export-pot commands to don't exports paragraph starting
with ":Usage:". I think this is not beautiful but enough.

>
> 58 - result += ':Purpose: %s\n' % purpose
> 59 + result += ':%s: %s\n' % (gettext('Purpose'), purpose)
>
> Hmm, this pattern will probably occurs quite frequently and I'm
> pretty sure the translators will sometimes need to tweak the
> format Is there some existing facility we can reuse so this can
> be spelt something like:
>
> result += gettext(':Purpose: %s\n' % (gettext(purpose),)
>
> or even better:
>
> result += gettext_xxx(':Purpose: %(purpose)s\n', dict(purpose=purpose))
>
> ?

OK, gettext(':Purpose: %s\n') % (gettext(purpose),) may be good.

>
> 74 + options = option.get_optparser(self.options(), True)
> 75 + options = options.format_option_help()
>
> How about:
>
> parser = option.get_optparser(self.options(), True)
> options = parser.format_option_help()
>
> 88 + if options.startswith('Options:') or options.startswith('options:'):
> 89 + # Python 2.4 version of optparse uses 'options'.
> 90 + result += ':%s:%s' % (gettext('Options'), options[len('options:'):])
>
> Forget about python-2.4, we don't support it anymore.
>

I'll fix.

> 135 - sys.stdout.write(self.get_help_text())
> 136 + self._setup_outf()
> 137 + self.outf.write(self.get_help_text())
> 138 return 0
> 139 if 'usage' in opts: # e.g. bzr add --usage
> 140 sys.stdout.write(self.get_help_text(verbose=False))
>
> /me blinks
>
> Nice to address this bug, but why on earth is it needed ? This
> should really be addressed far earlier no ? Can you find where or
> do you need help here ?

It was not problem before because get_help_text() returns bytestring
containing only ascii characters.
Since i18n is enabled, get_help_text() returns unicode though `outf`
is needed.

>
> Also, it looks like the exact same bug affect 'usage'. I realize
> this may be out of scope for this proposal, so may be we want a
> different proposal that, just let me know if you want to do it or
> leave it to me.

Ah, I've forgot to address --usage. I'll fix it.

>
>
> 145 + @staticmethod
> 146 + def get_gettext():
>
> I don't understand why you need a static method there ? I realize
> *you* don't need 'self' but may be some plugins will, no ?
>
> 149 + NOTE: Commands provided by plugins should override this to use own
> 150 + i18n system.
>
> typo/style nits: We don't use 'NOTE:' all caps, may be just 'Note:' or may be
> just nothing ;) s/to use own/to use their own/
>

I'll fix these.

> 165 + opt_no_i18n = opt_no_aliases = False
>
> Excuse my ignorance, but shouldn't that be opt_no_i10n here ?

Do you mean "l10n"?
I agree on "opt_no_l10n" or "opt_no_L10N" is better than "i10n".

>
> 196 + if p.splitlines()[0] == ':Usage:':
> 197 +...

Read more...

Revision history for this message
Vincent Ladeuil (vila) wrote :
Download full text (9.9 KiB)

>>>>> INADA Naoki <email address hidden> writes:

    >> 46 + # NOTE: If cmd_gettext translates ':Usage:\n', the section will
    >> 47 + # be shown after "Description" section.
    >> 48 + doc = cmd_gettext(doc)
    >>
    >> /me blinks
    >>
    >> I'm sure this will be eaiser to understand if you added a
    >> corresponding test.

    > Is the test for garbage input and garbage output really needed?

later you have:

221 + def filter(p):
222 + # ':Usage:' has special meaning in help topics.
223 + # This is usage example of command and should not be translated.
224 + if p.splitlines()[0] == ':Usage:':
225 + return True

Reading that, I think I understand. The later is needed to prepare the
file for translators while the former is making sure we don't try to
translate when we display the command help. Is that right ?

    > I've modified export-pot commands to don't exports paragraph
    > starting with ":Usage:". I think this is not beautiful but enough.

Indeed, now it makes sense, sorry for being a bit obtuse here :)

So I think what I wanted is:

    >> 46 + # NOTE: If cmd_gettext translates ':Usage:\n', the section will
    >> 47 + # be shown after "Description" section.

# be shown after "Description" section and we don't want to translate the usage string.

But we still want to translate 'Usage' itself right ? Like we want to
translate 'Description', 'Tips', 'See Also' and so on. Do we reuse these
specifics strings from somewhere or are they part of the strings our
translators will process ?

(Also see below for other related points)

<snip/>

    > OK, gettext(':Purpose: %s\n') % (gettext(purpose),) may be good.

Right, that looks like an easier idiom.

    >> 135 - sys.stdout.write(self.get_help_text())
    >> 136 + self._setup_outf()
    >> 137 + self.outf.write(self.get_help_text())
    >> 138 return 0
    >> 139 if 'usage' in opts: # e.g. bzr add --usage
    >> 140 sys.stdout.write(self.get_help_text(verbose=False))
    >>
    >> /me blinks
    >>
    >> Nice to address this bug, but why on earth is it needed ? This
    >> should really be addressed far earlier no ? Can you find where or
    >> do you need help here ?

    > It was not problem before because get_help_text() returns bytestring
    > containing only ascii characters.
    > Since i18n is enabled, get_help_text() returns unicode though `outf`
    > is needed.

Sorry, badly phrased. I understand why we need to use outf, I don't
understand why we need to call set_outf (or rather why it has not been
already called).

    >>
    >> Also, it looks like the exact same bug affect 'usage'. I realize
    >> this may be out of scope for this proposal, so may be we want a
    >> different proposal that, just let me know if you want to do it or
    >> leave it to me.

    > Ah, I've forgot to address --usage. I'll fix it.

Right, so same fix twice, hence my worries about why we need to fix that
as it seems to reveal a deeper issue...

    >>
    >>
    >> 145 + @staticmethod
    >> 146 + def get_gettext():
    >>
    >> I don't understand why you need a static method there ? I realize
    >> *you* don't need 's...

Revision history for this message
Martin Pool (mbp) wrote :

I'd like a second opinion but this looks good to me.

It may make future reviews faster if you can summarize the change in the mp
description.
On Jun 1, 2011 6:42 PM, "INADA Naoki" <email address hidden> wrote:
> INADA Naoki has proposed merging lp:~songofacandy/bzr/i18n-command into
lp:bzr.
>
> Requested reviews:
> Vincent Ladeuil (vila)
>
> For more details, see:
> https://code.launchpad.net/~songofacandy/bzr/i18n-command/+merge/62752
>
> Translate command help
> --
> https://code.launchpad.net/~songofacandy/bzr/i18n-command/+merge/62752
> Your team bzr-core is subscribed to branch lp:bzr.

Revision history for this message
methane (songofacandy) wrote :
Download full text (3.3 KiB)

> >>>>> INADA Naoki <email address hidden> writes:
>
> >> 46 + # NOTE: If cmd_gettext translates ':Usage:\n', the section
> will
> >> 47 + # be shown after "Description" section.
> >> 48 + doc = cmd_gettext(doc)
> >>
> >> /me blinks
> >>
> >> I'm sure this will be eaiser to understand if you added a
> >> corresponding test.
>
> > Is the test for garbage input and garbage output really needed?
>
> later you have:
>
> 221 + def filter(p):
> 222 + # ':Usage:' has special meaning in help topics.
> 223 + # This is usage example of command and should not be translated.
> 224 + if p.splitlines()[0] == ':Usage:':
> 225 + return True
>
> Reading that, I think I understand. The later is needed to prepare the
> file for translators while the former is making sure we don't try to
> translate when we display the command help. Is that right ?

Yes, I want to avoid translating ':Usage:' because it is used as a keyword
in Command.get_help_text().

>
> > I've modified export-pot commands to don't exports paragraph
> > starting with ":Usage:". I think this is not beautiful but enough.
>
> Indeed, now it makes sense, sorry for being a bit obtuse here :)
>
> So I think what I wanted is:
>
> >> 46 + # NOTE: If cmd_gettext translates ':Usage:\n', the section
> will
> >> 47 + # be shown after "Description" section.
>
> # be shown after "Description" section and we don't want to translate the
> usage string.

OK, I'll fix the comment.

>
> But we still want to translate 'Usage' itself right ? Like we want to
> translate 'Description', 'Tips', 'See Also' and so on. Do we reuse these
> specifics strings from somewhere or are they part of the strings our
> translators will process ?
>
> (Also see below for other related points)

Yes, get_help_text() removes ":Usage:" in the docstring and write separately.

> result += gettext(':Usage:\n%s') % (usage,) + '\n'

> >> 135 - sys.stdout.write(self.get_help_text())
> >> 136 + self._setup_outf()
> >> 137 + self.outf.write(self.get_help_text())
> >> 138 return 0
> >> 139 if 'usage' in opts: # e.g. bzr add --usage
> >> 140 sys.stdout.write(self.get_help_text(verbose=False))
> >>
> >> /me blinks
> >>
> >> Nice to address this bug, but why on earth is it needed ? This
> >> should really be addressed far earlier no ? Can you find where or
> >> do you need help here ?
>
> > It was not problem before because get_help_text() returns bytestring
> > containing only ascii characters.
> > Since i18n is enabled, get_help_text() returns unicode though `outf`
> > is needed.
>
> Sorry, badly phrased. I understand why we need to use outf, I don't
> understand why we need to call set_outf (or rather why it has not been
> already called).
>
> >>
> >> Also, it looks like the exact same bug affect 'usage'. I realize
> >> this may be out of scope for this proposal, so may be we want a
> >> different proposal that, just let me know if you want to do it or
> >> leave it to me.
>
> > Ah, I've forgot to address --usag...

Read more...

Revision history for this message
methane (songofacandy) wrote :

Then, I'll remove translating option help in this mp.

I'll add a new class that inherits OptionParser and overrides
format_option_help() in future mp.

Revision history for this message
Vincent Ladeuil (vila) wrote :

> Then, I'll remove translating option help in this mp.
>
> I'll add a new class that inherits OptionParser and overrides
> format_option_help() in future mp.

That sounds like the best way to move forward.

Mark this proposal NeedsReview when you're ready.

Revision history for this message
methane (songofacandy) wrote :

I think I've done what I said "I'll do it."
Anything else?

Revision history for this message
Vincent Ladeuil (vila) wrote :

>>>>> INADA Naoki writes:

    > I think I've done what I said "I'll do it."
    > Anything else?

Sorry for the delay I had some lag in my mail.

I'll look into it asap.

Revision history for this message
Vincent Ladeuil (vila) wrote :

So, the bad news first: 6 test failures:

bzrlib.tests.test_help.TestCommandHelpI18n.test_command_help_includes_see_also
bzrlib.tests.test_help.TestCommandHelpI18n.test_command_only_additional_see_also
bzrlib.tests.test_help.TestCommandHelpI18n.test_command_with_additional_see_also
bzrlib.tests.test_help.TestCommandHelpI18n.test_get_help_text
bzrlib.tests.test_help.TestCommandHelpI18n.test_help_custom_section_ordering
bzrlib.tests.test_help.TestCommandHelpI18n.test_help_text_custom_usage

I made some modifications to make these failures easier to diagnose (in lp:~vila/bzr/i18n-command) but you didn't merge them.

The good news now: I've merged these modifications again and what
happens is that your last commit broke them by installing the
"right" _translations (NullTranslation) when translating the
command help which defeats the installation done by the tests.

Also, these modifications included removing your modification
pattern:

60 - result += ':Purpose: %s\n' % purpose
61 + result += gettext(':Purpose: %s') % (purpose,) + '\n'

Specifically, I'm talking about removing the '\n' from the string
to be translated. If you have a strong reason to do so, let's put
it back and let's to do it everywhere it matters.

For i18n._translations, what we want is a lazy installation so
that it's done only once and that tests can force a specific
translation. This way, we can (as we do now) activate a
translation only when we need it: to generate the command help.

I went ahead and:

- initialize i18n._translation to None instead of NullTranslations,

- spell '_translations' with a final 's' (for consistency with
  gettext itself and ZzzTranslationsForDoc)

I'll land with these modifications and a news entry (please
review it and let me know what you think).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/builtins.py'
2--- bzrlib/builtins.py 2011-06-16 16:45:18 +0000
3+++ bzrlib/builtins.py 2011-06-23 16:06:27 +0000
4@@ -3728,6 +3728,7 @@
5 randomize=None, exclude=None, strict=False,
6 load_list=None, debugflag=None, starting_with=None, subunit=False,
7 parallel=None, lsprof_tests=False):
8+ from bzrlib import i18n
9 from bzrlib import tests
10
11 if testspecs_list is not None:
12
13=== modified file 'bzrlib/commands.py'
14--- bzrlib/commands.py 2011-05-19 09:32:38 +0000
15+++ bzrlib/commands.py 2011-06-23 16:06:27 +0000
16@@ -44,6 +44,11 @@
17 """)
18
19 from bzrlib.hooks import Hooks
20+from bzrlib.i18n import (
21+ gettext,
22+ gettext_per_paragraph,
23+ install as i18n_install,
24+ )
25 # Compatibility - Option used to be in commands.
26 from bzrlib.option import Option
27 from bzrlib.plugin import disable_plugins, load_plugins
28@@ -408,6 +413,7 @@
29 takes_options = []
30 encoding_type = 'strict'
31 invoked_as = None
32+ l10n = True
33
34 hidden = False
35
36@@ -486,8 +492,17 @@
37 message explaining how to obtain full help.
38 """
39 doc = self.help()
40- if not doc:
41- doc = "No help for this command."
42+ if doc:
43+ # Note: If self.gettext() translates ':Usage:\n', the section will
44+ # be shown after "Description" section and we don't want to
45+ # translate the usage string.
46+ # Though, bzr export-pot don't exports :Usage: section and it must
47+ # not be translated.
48+ if self.l10n:
49+ i18n_install() # Install i18n only for get_help_text for now.
50+ doc = self.gettext(doc)
51+ else:
52+ doc = gettext("No help for this command.")
53
54 # Extract the summary (purpose) and sections out from the text
55 purpose,sections,order = self._get_help_parts(doc)
56@@ -500,11 +515,11 @@
57
58 # The header is the purpose and usage
59 result = ""
60- result += ':Purpose: %s\n' % purpose
61+ result += gettext(':Purpose: %s') % (purpose,) + '\n'
62 if usage.find('\n') >= 0:
63- result += ':Usage:\n%s\n' % usage
64+ result += gettext(':Usage:\n%s') % (usage,) + '\n'
65 else:
66- result += ':Usage: %s\n' % usage
67+ result += gettext(':Usage: %s') % (usage,) + '\n'
68 result += '\n'
69
70 # Add the options
71@@ -512,7 +527,8 @@
72 # XXX: optparse implicitly rewraps the help, and not always perfectly,
73 # so we get <https://bugs.launchpad.net/bzr/+bug/249908>. -- mbp
74 # 20090319
75- options = option.get_optparser(self.options()).format_option_help()
76+ parser = option.get_optparser(self.options())
77+ options = parser.format_option_help()
78 # FIXME: According to the spec, ReST option lists actually don't
79 # support options like --1.14 so that causes syntax errors (in Sphinx
80 # at least). As that pattern always appears in the commands that
81@@ -522,10 +538,7 @@
82 if not plain and options.find(' --1.14 ') != -1:
83 options = options.replace(' format:\n', ' format::\n\n', 1)
84 if options.startswith('Options:'):
85- result += ':' + options
86- elif options.startswith('options:'):
87- # Python 2.4 version of optparse
88- result += ':Options:' + options[len('options:'):]
89+ result += gettext(':Options:%s') % (options[len('options:'):],)
90 else:
91 result += options
92 result += '\n'
93@@ -536,26 +549,26 @@
94 if sections.has_key(None):
95 text = sections.pop(None)
96 text = '\n '.join(text.splitlines())
97- result += ':%s:\n %s\n\n' % ('Description',text)
98+ result += gettext(':Description:\n %s') % (text,) + '\n\n'
99
100 # Add the custom sections (e.g. Examples). Note that there's no need
101 # to indent these as they must be indented already in the source.
102 if sections:
103 for label in order:
104- if sections.has_key(label):
105- result += ':%s:\n%s\n' % (label,sections[label])
106+ if label in sections:
107+ result += ':%s:\n%s\n' % (label, sections[label])
108 result += '\n'
109 else:
110- result += ("See bzr help %s for more details and examples.\n\n"
111+ result += (gettext("See bzr help %s for more details and examples.\n\n")
112 % self.name())
113
114 # Add the aliases, source (plug-in) and see also links, if any
115 if self.aliases:
116- result += ':Aliases: '
117+ result += gettext(':Aliases: ')
118 result += ', '.join(self.aliases) + '\n'
119 plugin_name = self.plugin_name()
120 if plugin_name is not None:
121- result += ':From: plugin "%s"\n' % plugin_name
122+ result += gettext(':From: plugin "%s"\n') % plugin_name
123 see_also = self.get_see_also(additional_see_also)
124 if see_also:
125 if not plain and see_also_as_links:
126@@ -567,11 +580,10 @@
127 see_also_links.append(item)
128 else:
129 # Use a Sphinx link for this entry
130- link_text = ":doc:`%s <%s-help>`" % (item, item)
131+ link_text = gettext(":doc:`%s <%s-help>`") % (item, item)
132 see_also_links.append(link_text)
133 see_also = see_also_links
134- result += ':See also: '
135- result += ', '.join(see_also) + '\n'
136+ result += gettext(':See also: %s') % ', '.join(see_also) + '\n'
137
138 # If this will be rendered as plain text, convert it
139 if plain:
140@@ -658,13 +670,14 @@
141 def run_argv_aliases(self, argv, alias_argv=None):
142 """Parse the command line and run with extra aliases in alias_argv."""
143 args, opts = parse_args(self, argv, alias_argv)
144+ self._setup_outf()
145
146 # Process the standard options
147 if 'help' in opts: # e.g. bzr add --help
148- sys.stdout.write(self.get_help_text())
149+ self.outf.write(self.get_help_text())
150 return 0
151 if 'usage' in opts: # e.g. bzr add --usage
152- sys.stdout.write(self.get_help_text(verbose=False))
153+ self.outf.write(self.get_help_text(verbose=False))
154 return 0
155 trace.set_verbosity_level(option._verbosity_level)
156 if 'verbose' in self.supported_std_options:
157@@ -685,8 +698,6 @@
158 all_cmd_args = cmdargs.copy()
159 all_cmd_args.update(cmdopts)
160
161- self._setup_outf()
162-
163 try:
164 return self.run(**all_cmd_args)
165 finally:
166@@ -751,6 +762,14 @@
167 return None
168 return getdoc(self)
169
170+ def gettext(self, message):
171+ """Returns the gettext function used to translate this command's help.
172+
173+ Commands provided by plugins should override this to use their
174+ own i18n system.
175+ """
176+ return gettext_per_paragraph(message)
177+
178 def name(self):
179 """Return the canonical name for this command.
180
181@@ -1041,8 +1060,8 @@
182 argv = _specified_or_unicode_argv(argv)
183 trace.mutter("bzr arguments: %r", argv)
184
185- opt_lsprof = opt_profile = opt_no_plugins = opt_builtin = \
186- opt_no_aliases = False
187+ opt_lsprof = opt_profile = opt_no_plugins = opt_builtin = \
188+ opt_no_l10n = opt_no_aliases = False
189 opt_lsprof_file = opt_coverage_dir = None
190
191 # --no-plugins is handled specially at a very early stage. We need
192@@ -1065,6 +1084,8 @@
193 opt_no_plugins = True
194 elif a == '--no-aliases':
195 opt_no_aliases = True
196+ elif a == '--no-l10n':
197+ opt_no_l10n = True
198 elif a == '--builtin':
199 opt_builtin = True
200 elif a == '--concurrency':
201@@ -1106,6 +1127,8 @@
202
203 cmd = argv.pop(0)
204 cmd_obj = get_cmd_object(cmd, plugins_override=not opt_builtin)
205+ if opt_no_l10n:
206+ cmd.l10n = False
207 run = cmd_obj.run_argv_aliases
208 run_argv = [argv, alias_argv]
209
210
211=== modified file 'bzrlib/export_pot.py'
212--- bzrlib/export_pot.py 2011-05-12 12:19:35 +0000
213+++ bzrlib/export_pot.py 2011-06-23 16:06:27 +0000
214@@ -76,10 +76,12 @@
215 'msgid %s\n' % _normalize(s) +
216 'msgstr ""\n')
217
218-def _poentry_per_paragraph(outf, path, lineno, msgid):
219+def _poentry_per_paragraph(outf, path, lineno, msgid, filter=lambda x: False):
220 # TODO: How to split long help?
221 paragraphs = msgid.split('\n\n')
222 for p in paragraphs:
223+ if filter(p):
224+ continue
225 _poentry(outf, path, lineno, p)
226 lineno += p.count('\n') + 2
227
228@@ -145,7 +147,7 @@
229 'help of %r option of %r command' % (name, cmd.name()))
230
231
232-def _write_command_help(outf, cmd_name, cmd):
233+def _write_command_help(outf, cmd):
234 path = inspect.getfile(cmd.__class__)
235 if path.endswith('.pyc'):
236 path = path[:-1]
237@@ -155,9 +157,16 @@
238 lineno = offsets[cmd.__doc__]
239 doc = inspect.getdoc(cmd)
240
241- _poentry_per_paragraph(outf, path, lineno, doc)
242+ def filter(p):
243+ # ':Usage:' has special meaning in help topics.
244+ # This is usage example of command and should not be translated.
245+ if p.splitlines()[0] == ':Usage:':
246+ return True
247+
248+ _poentry_per_paragraph(outf, path, lineno, doc, filter)
249 _command_options(outf, path, cmd)
250
251+
252 def _command_helps(outf):
253 """Extract docstrings from path.
254
255@@ -172,7 +181,7 @@
256 if command.hidden:
257 continue
258 note("Exporting messages from builtin command: %s", cmd_name)
259- _write_command_help(outf, cmd_name, command)
260+ _write_command_help(outf, command)
261
262 plugin_path = plugin.get_core_plugin_path()
263 core_plugins = glob(plugin_path + '/*/__init__.py')
264@@ -189,7 +198,7 @@
265 continue
266 note("Exporting messages from plugin command: %s in %s",
267 cmd_name, command.plugin_name())
268- _write_command_help(outf, cmd_name, command)
269+ _write_command_help(outf, command)
270
271
272 def _error_messages(outf):
273
274=== modified file 'bzrlib/help.py'
275--- bzrlib/help.py 2011-05-11 16:52:02 +0000
276+++ bzrlib/help.py 2011-06-23 16:06:27 +0000
277@@ -22,7 +22,6 @@
278 # TODO: `help commands --all` should show hidden commands
279
280 import sys
281-import textwrap
282
283 from bzrlib import (
284 commands as _mod_commands,
285@@ -30,6 +29,7 @@
286 help_topics,
287 osutils,
288 plugin,
289+ utextwrap,
290 )
291
292
293@@ -96,7 +96,7 @@
294 else:
295 firstline = ''
296 helpstring = '%-*s %s%s' % (max_name, cmd_name, firstline, plugin_name)
297- lines = textwrap.wrap(
298+ lines = utextwrap.wrap(
299 helpstring, subsequent_indent=indent,
300 width=width,
301 break_long_words=False)
302
303=== modified file 'bzrlib/help_topics/__init__.py'
304--- bzrlib/help_topics/__init__.py 2010-11-06 10:55:58 +0000
305+++ bzrlib/help_topics/__init__.py 2011-06-23 16:06:27 +0000
306@@ -316,6 +316,7 @@
307 --builtin Use the built-in version of a command, not the plugin version.
308 This does not suppress other plugin effects.
309 --no-plugins Do not process any plugins.
310+--no-l10n Do not translate messages.
311 --concurrency Number of processes that can be run concurrently (selftest).
312
313 --profile Profile execution using the hotshot profiler.
314
315=== modified file 'bzrlib/i18n.py'
316--- bzrlib/i18n.py 2011-05-27 21:38:33 +0000
317+++ bzrlib/i18n.py 2011-06-23 16:06:27 +0000
318@@ -75,7 +75,7 @@
319
320 def uninstall():
321 global _translation
322- _translation = _null_translation
323+ _translation = _gettext.NullTranslations()
324
325
326 def _get_locale_dir():
327
328=== modified file 'bzrlib/tests/test_export_pot.py'
329--- bzrlib/tests/test_export_pot.py 2011-05-11 17:44:45 +0000
330+++ bzrlib/tests/test_export_pot.py 2011-06-23 16:06:27 +0000
331@@ -18,10 +18,14 @@
332 import textwrap
333
334 from bzrlib import (
335+ commands,
336 export_pot,
337 tests,
338 )
339
340+import re
341+
342+
343 class TestEscape(tests.TestCase):
344
345 def test_simple_escape(self):
346@@ -145,3 +149,45 @@
347 "EGG\\n"
348 msgstr ""\n
349 ''')
350+
351+
352+class TestExportCommandHelp(PoEntryTestCase):
353+
354+ def test_command_help(self):
355+
356+ class cmd_Demo(commands.Command):
357+ __doc__ = """A sample command.
358+
359+ :Usage:
360+ bzr demo
361+
362+ :Examples:
363+ Example 1::
364+
365+ cmd arg1
366+
367+ Blah Blah Blah
368+ """
369+
370+ export_pot._write_command_help(self._outf, cmd_Demo())
371+ result = self._outf.getvalue()
372+ # We don't concern abount filename and lineno here.
373+ result = re.sub(r'^#: [^\n]+\n', '', result, flags=re.MULTILINE)
374+
375+ self.assertEqualDiff(
376+ 'msgid "A sample command."\n'
377+ 'msgstr ""\n'
378+ '\n' # :Usage: should not be translated.
379+ 'msgid ""\n'
380+ '":Examples:\\n"\n'
381+ '" Example 1::"\n'
382+ 'msgstr ""\n'
383+ '\n'
384+ 'msgid " cmd arg1"\n'
385+ 'msgstr ""\n'
386+ '\n'
387+ 'msgid "Blah Blah Blah"\n'
388+ 'msgstr ""\n'
389+ '\n',
390+ result
391+ )
392
393=== modified file 'bzrlib/tests/test_help.py'
394--- bzrlib/tests/test_help.py 2011-01-12 01:01:53 +0000
395+++ bzrlib/tests/test_help.py 2011-06-23 16:06:27 +0000
396@@ -22,10 +22,14 @@
397 errors,
398 help,
399 help_topics,
400+ i18n,
401 plugin,
402 tests,
403 )
404
405+from bzrlib.tests.test_i18n import ZzzTranslations
406+import re
407+
408
409 class TestHelp(tests.TestCase):
410
411@@ -295,6 +299,161 @@
412 ' Blah blah blah.\n\n')
413
414
415+class ZzzTranslationsForDoc(ZzzTranslations):
416+
417+ _section_pat = re.compile(':\w+:\\n\\s+')
418+ _indent_pat = re.compile('\\s+')
419+
420+ def zzz(self, s):
421+ m = self._section_pat.match(s)
422+ if m is None:
423+ m = self._indent_pat.match(s)
424+ if m:
425+ return u'%szz{{%s}}' % (m.group(0), s[m.end():])
426+ return u'zz{{%s}}' % s
427+
428+
429+class TestCommandHelpI18n(tests.TestCase):
430+ """Tests for help on translated commands."""
431+
432+ def setUp(self):
433+ super(TestCommandHelpI18n, self).setUp()
434+ self.overrideAttr(i18n, '_translation', ZzzTranslationsForDoc())
435+
436+ def test_command_help_includes_see_also(self):
437+ class cmd_WithSeeAlso(commands.Command):
438+ __doc__ = """A sample command."""
439+ _see_also = ['foo', 'bar']
440+ cmd = cmd_WithSeeAlso()
441+ helptext = cmd.get_help_text()
442+ self.assertEndsWith(
443+ helptext,
444+ ' -v, --verbose Display more information.\n'
445+ ' -q, --quiet Only display errors and warnings.\n'
446+ ' -h, --help Show help message.\n'
447+ '}}\n'
448+ 'zz{{:See also: bar, foo}}\n')
449+
450+ def test_get_help_text(self):
451+ """Commands have a get_help_text method which returns their help."""
452+ class cmd_Demo(commands.Command):
453+ __doc__ = """A sample command."""
454+ cmd = cmd_Demo()
455+ helptext = cmd.get_help_text()
456+ self.assertStartsWith(helptext,
457+ 'zz{{:Purpose: zz{{A sample command.}}}}\n'
458+ 'zz{{:Usage: bzr Demo}}\n')
459+ self.assertEndsWith(helptext,
460+ ' -h, --help Show help message.\n}}\n')
461+
462+ def test_command_with_additional_see_also(self):
463+ class cmd_WithSeeAlso(commands.Command):
464+ __doc__ = """A sample command."""
465+ _see_also = ['foo', 'bar']
466+ cmd = cmd_WithSeeAlso()
467+ helptext = cmd.get_help_text(['gam'])
468+ self.assertEndsWith(
469+ helptext,
470+ ' -v, --verbose Display more information.\n'
471+ ' -q, --quiet Only display errors and warnings.\n'
472+ ' -h, --help Show help message.\n'
473+ '}}\n'
474+ 'zz{{:See also: bar, foo, gam}}\n')
475+
476+ def test_command_only_additional_see_also(self):
477+ class cmd_WithSeeAlso(commands.Command):
478+ __doc__ = """A sample command."""
479+ cmd = cmd_WithSeeAlso()
480+ helptext = cmd.get_help_text(['gam'])
481+ self.assertEndsWith(
482+ helptext,
483+ 'zz{{:Options:\n'
484+ ' --usage Show usage message and options.\n'
485+ ' -v, --verbose Display more information.\n'
486+ ' -q, --quiet Only display errors and warnings.\n'
487+ ' -h, --help Show help message.\n'
488+ '}}\n'
489+ 'zz{{:See also: gam}}\n')
490+
491+
492+ def test_help_custom_section_ordering(self):
493+ """Custom descriptive sections should remain in the order given."""
494+ class cmd_Demo(commands.Command):
495+ __doc__ = """A sample command.
496+
497+ Blah blah blah.
498+
499+ :Formats:
500+ Interesting stuff about formats.
501+
502+ :Examples:
503+ Example 1::
504+
505+ cmd arg1
506+
507+ :Tips:
508+ Clever things to keep in mind.
509+ """
510+ cmd = cmd_Demo()
511+ helptext = cmd.get_help_text()
512+ self.assertEqualDiff(
513+ helptext,
514+ 'zz{{:Purpose: zz{{A sample command.}}}}\n'
515+ 'zz{{:Usage: bzr Demo}}\n'
516+ '\n'
517+ 'zz{{:Options:\n'
518+ ' --usage Show usage message and options.\n'
519+ ' -v, --verbose Display more information.\n'
520+ ' -q, --quiet Only display errors and warnings.\n'
521+ ' -h, --help Show help message.\n'
522+ '}}\n'
523+ 'Description:\n'
524+ ' zz{{zz{{Blah blah blah.}}}}\n'
525+ '\n'
526+ 'Formats:\n'
527+ ' zz{{Interesting stuff about formats.}}\n'
528+ '\n'
529+ 'Examples:\n'
530+ ' zz{{Example 1::}}\n'
531+ '\n'
532+ ' zz{{cmd arg1}}\n'
533+ '\n'
534+ 'Tips:\n'
535+ ' zz{{Clever things to keep in mind.}}\n'
536+ '\n')
537+
538+ def test_help_text_custom_usage(self):
539+ """Help text may contain a custom usage section."""
540+ class cmd_Demo(commands.Command):
541+ __doc__ = """A sample command.
542+
543+ :Usage:
544+ cmd Demo [opts] args
545+
546+ cmd Demo -h
547+
548+ Blah blah blah.
549+ """
550+ cmd = cmd_Demo()
551+ helptext = cmd.get_help_text()
552+ self.assertEquals(helptext,
553+ 'zz{{:Purpose: zz{{A sample command.}}}}\n'
554+ 'zz{{:Usage:\n'
555+ ' zz{{cmd Demo [opts] args}}\n'
556+ '\n'
557+ ' zz{{cmd Demo -h}}\n'
558+ '}}\n'
559+ '\n'
560+ 'zz{{:Options:\n'
561+ ' --usage Show usage message and options.\n'
562+ ' -v, --verbose Display more information.\n'
563+ ' -q, --quiet Only display errors and warnings.\n'
564+ ' -h, --help Show help message.\n'
565+ '}}\n'
566+ 'Description:\n'
567+ ' zz{{zz{{Blah blah blah.}}}}\n\n')
568+
569+
570 class TestRegisteredTopic(TestHelp):
571 """Tests for the RegisteredTopic class."""
572