Merge lp:~vila/bzr/747050-config-help into lp:bzr
- 747050-config-help
- Merge into bzr.dev
Proposed by
Vincent Ladeuil
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Jelmer Vernooij | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 6072 | ||||
Proposed branch: | lp:~vila/bzr/747050-config-help | ||||
Merge into: | lp:bzr | ||||
Diff against target: |
323 lines (+103/-45) 8 files modified
bzrlib/config.py (+16/-8) bzrlib/help.py (+2/-1) bzrlib/help_topics/__init__.py (+39/-14) bzrlib/help_topics/en/configuration.txt (+2/-2) bzrlib/plugin.py (+3/-12) bzrlib/tests/test_help.py (+34/-5) bzrlib/tests/test_plugins.py (+4/-3) doc/en/release-notes/bzr-2.5.txt (+3/-0) |
||||
To merge this branch: | bzr merge lp:~vila/bzr/747050-config-help | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jelmer Vernooij (community) | Needs Fixing | ||
Review via email: mp+70929@code.launchpad.net |
Commit message
Implement per-config option help
Description of the change
This implements help topics for registered configuration options.
I refrained from refactoring too much (still reducing the code
duplication a bit) but some love seems to be needed in this area.
I haven't deleted the text from help_topics/
yet, that would be better addressed when fixing bug #746993.
To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote : | # |
sent to pqm by email
Revision history for this message
Vincent Ladeuil (vila) wrote : | # |
sent to pqm by email
Revision history for this message
Vincent Ladeuil (vila) wrote : | # |
sent to pqm by email
Revision history for this message
Vincent Ladeuil (vila) wrote : | # |
sent to pqm by email
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'bzrlib/config.py' | |||
2 | --- bzrlib/config.py 2011-08-13 03:39:24 +0000 | |||
3 | +++ bzrlib/config.py 2011-08-16 07:35:26 +0000 | |||
4 | @@ -2307,6 +2307,15 @@ | |||
5 | 2307 | def get_default(self): | 2307 | def get_default(self): |
6 | 2308 | return self.default | 2308 | return self.default |
7 | 2309 | 2309 | ||
8 | 2310 | def get_help_text(self, additional_see_also=None, plain=True): | ||
9 | 2311 | result = self.help | ||
10 | 2312 | from bzrlib import help_topics | ||
11 | 2313 | result += help_topics._format_see_also(additional_see_also) | ||
12 | 2314 | if plain: | ||
13 | 2315 | result = help_topics.help_as_plain_text(result) | ||
14 | 2316 | return result | ||
15 | 2317 | |||
16 | 2318 | |||
17 | 2310 | # Predefined converters to get proper values from store | 2319 | # Predefined converters to get proper values from store |
18 | 2311 | 2320 | ||
19 | 2312 | def bool_from_store(unicode_str): | 2321 | def bool_from_store(unicode_str): |
20 | @@ -2351,13 +2360,12 @@ | |||
21 | 2351 | def register_lazy(self, key, module_name, member_name): | 2360 | def register_lazy(self, key, module_name, member_name): |
22 | 2352 | """Register a new option to be loaded on request. | 2361 | """Register a new option to be loaded on request. |
23 | 2353 | 2362 | ||
31 | 2354 | :param key: This is the key to use to request the option later. Since | 2363 | :param key: the key to request the option later. Since the registration |
32 | 2355 | the registration is lazy, it should be provided and match the | 2364 | is lazy, it should be provided and match the option name. |
33 | 2356 | option name. | 2365 | |
34 | 2357 | 2366 | :param module_name: the python path to the module. Such as 'os.path'. | |
35 | 2358 | :param module_name: The python path to the module. Such as 'os.path'. | 2367 | |
36 | 2359 | 2368 | :param member_name: the member of the module to return. If empty or | |
30 | 2360 | :param member_name: The member of the module to return. If empty or | ||
37 | 2361 | None, get() will return the module itself. | 2369 | None, get() will return the module itself. |
38 | 2362 | """ | 2370 | """ |
39 | 2363 | super(OptionRegistry, self).register_lazy(key, | 2371 | super(OptionRegistry, self).register_lazy(key, |
40 | @@ -2379,7 +2387,7 @@ | |||
41 | 2379 | 2387 | ||
42 | 2380 | option_registry.register( | 2388 | option_registry.register( |
43 | 2381 | Option('dirstate.fdatasync', default=True, from_unicode=bool_from_store, | 2389 | Option('dirstate.fdatasync', default=True, from_unicode=bool_from_store, |
45 | 2382 | help=''' | 2390 | help='''\ |
46 | 2383 | Flush dirstate changes onto physical disk? | 2391 | Flush dirstate changes onto physical disk? |
47 | 2384 | 2392 | ||
48 | 2385 | If true (default), working tree metadata changes are flushed through the | 2393 | If true (default), working tree metadata changes are flushed through the |
49 | 2386 | 2394 | ||
50 | === modified file 'bzrlib/help.py' | |||
51 | --- bzrlib/help.py 2011-05-18 18:32:22 +0000 | |||
52 | +++ bzrlib/help.py 2011-08-16 07:35:26 +0000 | |||
53 | @@ -1,4 +1,4 @@ | |||
55 | 1 | # Copyright (C) 2005-2010 Canonical Ltd | 1 | # Copyright (C) 2005-2011 Canonical Ltd |
56 | 2 | # | 2 | # |
57 | 3 | # This program is free software; you can redistribute it and/or modify | 3 | # This program is free software; you can redistribute it and/or modify |
58 | 4 | # it under the terms of the GNU General Public License as published by | 4 | # it under the terms of the GNU General Public License as published by |
59 | @@ -135,6 +135,7 @@ | |||
60 | 135 | help_topics.HelpTopicIndex(), | 135 | help_topics.HelpTopicIndex(), |
61 | 136 | _mod_commands.HelpCommandIndex(), | 136 | _mod_commands.HelpCommandIndex(), |
62 | 137 | plugin.PluginsHelpIndex(), | 137 | plugin.PluginsHelpIndex(), |
63 | 138 | help_topics.ConfigOptionHelpIndex(), | ||
64 | 138 | ] | 139 | ] |
65 | 139 | 140 | ||
66 | 140 | def _check_prefix_uniqueness(self): | 141 | def _check_prefix_uniqueness(self): |
67 | 141 | 142 | ||
68 | === modified file 'bzrlib/help_topics/__init__.py' | |||
69 | --- bzrlib/help_topics/__init__.py 2011-06-30 16:47:06 +0000 | |||
70 | +++ bzrlib/help_topics/__init__.py 2011-08-16 07:35:26 +0000 | |||
71 | @@ -1,4 +1,4 @@ | |||
73 | 1 | # Copyright (C) 2006-2010 Canonical Ltd | 1 | # Copyright (C) 2006-2011 Canonical Ltd |
74 | 2 | # | 2 | # |
75 | 3 | # This program is free software; you can redistribute it and/or modify | 3 | # This program is free software; you can redistribute it and/or modify |
76 | 4 | # it under the terms of the GNU General Public License as published by | 4 | # it under the terms of the GNU General Public License as published by |
77 | @@ -37,6 +37,7 @@ | |||
78 | 37 | 37 | ||
79 | 38 | import bzrlib | 38 | import bzrlib |
80 | 39 | from bzrlib import ( | 39 | from bzrlib import ( |
81 | 40 | config, | ||
82 | 40 | osutils, | 41 | osutils, |
83 | 41 | registry, | 42 | registry, |
84 | 42 | ) | 43 | ) |
85 | @@ -65,7 +66,7 @@ | |||
86 | 65 | :param section: Section in reference manual - see SECT_* identifiers. | 66 | :param section: Section in reference manual - see SECT_* identifiers. |
87 | 66 | """ | 67 | """ |
88 | 67 | # The detail is stored as the 'object' and the metadata as the info | 68 | # The detail is stored as the 'object' and the metadata as the info |
90 | 68 | info=(summary,section) | 69 | info = (summary, section) |
91 | 69 | super(HelpTopicRegistry, self).register(topic, detail, info=info) | 70 | super(HelpTopicRegistry, self).register(topic, detail, info=info) |
92 | 70 | 71 | ||
93 | 71 | def register_lazy(self, topic, module_name, member_name, summary, | 72 | def register_lazy(self, topic, module_name, member_name, summary, |
94 | @@ -79,7 +80,7 @@ | |||
95 | 79 | :param section: Section in reference manual - see SECT_* identifiers. | 80 | :param section: Section in reference manual - see SECT_* identifiers. |
96 | 80 | """ | 81 | """ |
97 | 81 | # The detail is stored as the 'object' and the metadata as the info | 82 | # The detail is stored as the 'object' and the metadata as the info |
99 | 82 | info=(summary,section) | 83 | info = (summary, section) |
100 | 83 | super(HelpTopicRegistry, self).register_lazy(topic, module_name, | 84 | super(HelpTopicRegistry, self).register_lazy(topic, module_name, |
101 | 84 | member_name, info=info) | 85 | member_name, info=info) |
102 | 85 | 86 | ||
103 | @@ -844,6 +845,15 @@ | |||
104 | 844 | return [] | 845 | return [] |
105 | 845 | 846 | ||
106 | 846 | 847 | ||
107 | 848 | def _format_see_also(see_also): | ||
108 | 849 | result = '' | ||
109 | 850 | if see_also: | ||
110 | 851 | result += '\n:See also: ' | ||
111 | 852 | result += ', '.join(sorted(set(see_also))) | ||
112 | 853 | result += '\n' | ||
113 | 854 | return result | ||
114 | 855 | |||
115 | 856 | |||
116 | 847 | class RegisteredTopic(object): | 857 | class RegisteredTopic(object): |
117 | 848 | """A help topic which has been registered in the HelpTopicRegistry. | 858 | """A help topic which has been registered in the HelpTopicRegistry. |
118 | 849 | 859 | ||
119 | @@ -867,17 +877,7 @@ | |||
120 | 867 | returned instead of plain text. | 877 | returned instead of plain text. |
121 | 868 | """ | 878 | """ |
122 | 869 | result = topic_registry.get_detail(self.topic) | 879 | result = topic_registry.get_detail(self.topic) |
134 | 870 | # there is code duplicated here and in bzrlib/plugin.py's | 880 | result += _format_see_also(additional_see_also) |
124 | 871 | # matching Topic code. This should probably be factored in | ||
125 | 872 | # to a helper function and a common base class. | ||
126 | 873 | if additional_see_also is not None: | ||
127 | 874 | see_also = sorted(set(additional_see_also)) | ||
128 | 875 | else: | ||
129 | 876 | see_also = None | ||
130 | 877 | if see_also: | ||
131 | 878 | result += '\n:See also: ' | ||
132 | 879 | result += ', '.join(see_also) | ||
133 | 880 | result += '\n' | ||
135 | 881 | if plain: | 881 | if plain: |
136 | 882 | result = help_as_plain_text(result) | 882 | result = help_as_plain_text(result) |
137 | 883 | return result | 883 | return result |
138 | @@ -903,3 +903,28 @@ | |||
139 | 903 | line = re.sub(":doc:`(.+?)-help`", r'``bzr help \1``', line) | 903 | line = re.sub(":doc:`(.+?)-help`", r'``bzr help \1``', line) |
140 | 904 | result.append(line) | 904 | result.append(line) |
141 | 905 | return "\n".join(result) + "\n" | 905 | return "\n".join(result) + "\n" |
142 | 906 | |||
143 | 907 | |||
144 | 908 | class ConfigOptionHelpIndex(object): | ||
145 | 909 | """A help index that returns help topics for config options.""" | ||
146 | 910 | |||
147 | 911 | def __init__(self): | ||
148 | 912 | self.prefix = 'configuration/' | ||
149 | 913 | |||
150 | 914 | def get_topics(self, topic): | ||
151 | 915 | """Search for topic in the registered config options. | ||
152 | 916 | |||
153 | 917 | :param topic: A topic to search for. | ||
154 | 918 | :return: A list which is either empty or contains a single | ||
155 | 919 | config.Option entry. | ||
156 | 920 | """ | ||
157 | 921 | if topic is None: | ||
158 | 922 | return [] | ||
159 | 923 | elif topic.startswith(self.prefix): | ||
160 | 924 | topic = topic[len(self.prefix):] | ||
161 | 925 | if topic in config.option_registry: | ||
162 | 926 | return [config.option_registry.get(topic)] | ||
163 | 927 | else: | ||
164 | 928 | return [] | ||
165 | 929 | |||
166 | 930 | |||
167 | 906 | 931 | ||
168 | === modified file 'bzrlib/help_topics/en/configuration.txt' | |||
169 | --- bzrlib/help_topics/en/configuration.txt 2011-08-02 01:10:27 +0000 | |||
170 | +++ bzrlib/help_topics/en/configuration.txt 2011-08-16 07:35:26 +0000 | |||
171 | @@ -14,7 +14,7 @@ | |||
172 | 14 | 14 | ||
173 | 15 | "John Doe <jdoe@example.com>" | 15 | "John Doe <jdoe@example.com>" |
174 | 16 | 16 | ||
176 | 17 | See also the ``email`` configuration value. | 17 | See also the ``email`` configuration option. |
177 | 18 | 18 | ||
178 | 19 | BZR_PROGRESS_BAR | 19 | BZR_PROGRESS_BAR |
179 | 20 | ~~~~~~~~~~~~~~~~ | 20 | ~~~~~~~~~~~~~~~~ |
180 | @@ -54,7 +54,7 @@ | |||
181 | 54 | 54 | ||
182 | 55 | Path to the Bazaar executable to use when using the bzr+ssh protocol. | 55 | Path to the Bazaar executable to use when using the bzr+ssh protocol. |
183 | 56 | 56 | ||
185 | 57 | See also the ``bzr_remote_path`` configuration value. | 57 | See also the ``bzr_remote_path`` configuration option. |
186 | 58 | 58 | ||
187 | 59 | BZR_EDITOR | 59 | BZR_EDITOR |
188 | 60 | ~~~~~~~~~~ | 60 | ~~~~~~~~~~ |
189 | 61 | 61 | ||
190 | === modified file 'bzrlib/plugin.py' | |||
191 | --- bzrlib/plugin.py 2011-05-31 06:15:24 +0000 | |||
192 | +++ bzrlib/plugin.py 2011-08-16 07:35:26 +0000 | |||
193 | @@ -506,21 +506,12 @@ | |||
194 | 506 | result = self.module.__doc__ | 506 | result = self.module.__doc__ |
195 | 507 | if result[-1] != '\n': | 507 | if result[-1] != '\n': |
196 | 508 | result += '\n' | 508 | result += '\n' |
208 | 509 | # there is code duplicated here and in bzrlib/help_topic.py's | 509 | from bzrlib import help_topics |
209 | 510 | # matching Topic code. This should probably be factored in | 510 | result += help_topics._format_see_also(additional_see_also) |
199 | 511 | # to a helper function and a common base class. | ||
200 | 512 | if additional_see_also is not None: | ||
201 | 513 | see_also = sorted(set(additional_see_also)) | ||
202 | 514 | else: | ||
203 | 515 | see_also = None | ||
204 | 516 | if see_also: | ||
205 | 517 | result += 'See also: ' | ||
206 | 518 | result += ', '.join(see_also) | ||
207 | 519 | result += '\n' | ||
210 | 520 | return result | 511 | return result |
211 | 521 | 512 | ||
212 | 522 | def get_help_topic(self): | 513 | def get_help_topic(self): |
214 | 523 | """Return the modules help topic - its __name__ after bzrlib.plugins..""" | 514 | """Return the module help topic: its basename.""" |
215 | 524 | return self.module.__name__[len('bzrlib.plugins.'):] | 515 | return self.module.__name__[len('bzrlib.plugins.'):] |
216 | 525 | 516 | ||
217 | 526 | 517 | ||
218 | 527 | 518 | ||
219 | === modified file 'bzrlib/tests/test_help.py' | |||
220 | --- bzrlib/tests/test_help.py 2011-06-27 15:36:58 +0000 | |||
221 | +++ bzrlib/tests/test_help.py 2011-08-16 07:35:26 +0000 | |||
222 | @@ -21,6 +21,7 @@ | |||
223 | 21 | from bzrlib import ( | 21 | from bzrlib import ( |
224 | 22 | builtins, | 22 | builtins, |
225 | 23 | commands, | 23 | commands, |
226 | 24 | config, | ||
227 | 24 | errors, | 25 | errors, |
228 | 25 | help, | 26 | help, |
229 | 26 | help_topics, | 27 | help_topics, |
230 | @@ -561,6 +562,31 @@ | |||
231 | 561 | self.assertEqual('', index.prefix) | 562 | self.assertEqual('', index.prefix) |
232 | 562 | 563 | ||
233 | 563 | 564 | ||
234 | 565 | class TestConfigOptionIndex(TestHelp): | ||
235 | 566 | """Tests for the HelpCommandIndex class.""" | ||
236 | 567 | |||
237 | 568 | def setUp(self): | ||
238 | 569 | super(TestConfigOptionIndex, self).setUp() | ||
239 | 570 | self.index = help_topics.ConfigOptionHelpIndex() | ||
240 | 571 | |||
241 | 572 | def test_get_topics_None(self): | ||
242 | 573 | """Searching for None returns an empty list.""" | ||
243 | 574 | self.assertEqual([], self.index.get_topics(None)) | ||
244 | 575 | |||
245 | 576 | def test_get_topics_no_topic(self): | ||
246 | 577 | self.assertEqual([], self.index.get_topics('nothing by this name')) | ||
247 | 578 | |||
248 | 579 | def test_prefix(self): | ||
249 | 580 | self.assertEqual('configuration/', self.index.prefix) | ||
250 | 581 | |||
251 | 582 | def test_get_topic_with_prefix(self): | ||
252 | 583 | topics = self.index.get_topics('configuration/default_format') | ||
253 | 584 | self.assertLength(1, topics) | ||
254 | 585 | opt = topics[0] | ||
255 | 586 | self.assertIsInstance(opt, config.Option) | ||
256 | 587 | self.assertEquals('default_format', opt.name) | ||
257 | 588 | |||
258 | 589 | |||
259 | 564 | class TestCommandIndex(TestHelp): | 590 | class TestCommandIndex(TestHelp): |
260 | 565 | """Tests for the HelpCommandIndex class.""" | 591 | """Tests for the HelpCommandIndex class.""" |
261 | 566 | 592 | ||
262 | @@ -603,16 +629,19 @@ | |||
263 | 603 | def test_default_search_path(self): | 629 | def test_default_search_path(self): |
264 | 604 | """The default search path should include internal indexs.""" | 630 | """The default search path should include internal indexs.""" |
265 | 605 | indices = help.HelpIndices() | 631 | indices = help.HelpIndices() |
267 | 606 | self.assertEqual(3, len(indices.search_path)) | 632 | self.assertEqual(4, len(indices.search_path)) |
268 | 607 | # help topics should be searched in first. | 633 | # help topics should be searched in first. |
269 | 608 | self.assertIsInstance(indices.search_path[0], | 634 | self.assertIsInstance(indices.search_path[0], |
271 | 609 | help_topics.HelpTopicIndex) | 635 | help_topics.HelpTopicIndex) |
272 | 610 | # with commands being search second. | 636 | # with commands being search second. |
273 | 611 | self.assertIsInstance(indices.search_path[1], | 637 | self.assertIsInstance(indices.search_path[1], |
276 | 612 | commands.HelpCommandIndex) | 638 | commands.HelpCommandIndex) |
277 | 613 | # and plugins are a third index. | 639 | # plugins are a third index. |
278 | 614 | self.assertIsInstance(indices.search_path[2], | 640 | self.assertIsInstance(indices.search_path[2], |
280 | 615 | plugin.PluginsHelpIndex) | 641 | plugin.PluginsHelpIndex) |
281 | 642 | # config options are a fourth index | ||
282 | 643 | self.assertIsInstance(indices.search_path[3], | ||
283 | 644 | help_topics.ConfigOptionHelpIndex) | ||
284 | 616 | 645 | ||
285 | 617 | def test_search_for_unknown_topic_raises(self): | 646 | def test_search_for_unknown_topic_raises(self): |
286 | 618 | """Searching for an unknown topic should raise NoHelpTopic.""" | 647 | """Searching for an unknown topic should raise NoHelpTopic.""" |
287 | 619 | 648 | ||
288 | === modified file 'bzrlib/tests/test_plugins.py' | |||
289 | --- bzrlib/tests/test_plugins.py 2011-05-16 13:49:58 +0000 | |||
290 | +++ bzrlib/tests/test_plugins.py 2011-08-16 07:35:26 +0000 | |||
291 | @@ -615,15 +615,16 @@ | |||
292 | 615 | def test_get_help_text_with_additional_see_also(self): | 615 | def test_get_help_text_with_additional_see_also(self): |
293 | 616 | mod = FakeModule('two lines of help\nand more', 'demo') | 616 | mod = FakeModule('two lines of help\nand more', 'demo') |
294 | 617 | topic = plugin.ModuleHelpTopic(mod) | 617 | topic = plugin.ModuleHelpTopic(mod) |
297 | 618 | self.assertEqual("two lines of help\nand more\nSee also: bar, foo\n", | 618 | self.assertEqual("two lines of help\nand more\n\n:See also: bar, foo\n", |
298 | 619 | topic.get_help_text(['foo', 'bar'])) | 619 | topic.get_help_text(['foo', 'bar'])) |
299 | 620 | 620 | ||
300 | 621 | def test_get_help_topic(self): | 621 | def test_get_help_topic(self): |
301 | 622 | """The help topic for a plugin is its module name.""" | 622 | """The help topic for a plugin is its module name.""" |
302 | 623 | mod = FakeModule('two lines of help\nand more', 'bzrlib.plugins.demo') | 623 | mod = FakeModule('two lines of help\nand more', 'bzrlib.plugins.demo') |
303 | 624 | topic = plugin.ModuleHelpTopic(mod) | 624 | topic = plugin.ModuleHelpTopic(mod) |
304 | 625 | self.assertEqual('demo', topic.get_help_topic()) | 625 | self.assertEqual('demo', topic.get_help_topic()) |
306 | 626 | mod = FakeModule('two lines of help\nand more', 'bzrlib.plugins.foo_bar') | 626 | mod = FakeModule('two lines of help\nand more', |
307 | 627 | 'bzrlib.plugins.foo_bar') | ||
308 | 627 | topic = plugin.ModuleHelpTopic(mod) | 628 | topic = plugin.ModuleHelpTopic(mod) |
309 | 628 | self.assertEqual('foo_bar', topic.get_help_topic()) | 629 | self.assertEqual('foo_bar', topic.get_help_topic()) |
310 | 629 | 630 | ||
311 | 630 | 631 | ||
312 | === modified file 'doc/en/release-notes/bzr-2.5.txt' | |||
313 | --- doc/en/release-notes/bzr-2.5.txt 2011-08-15 13:49:38 +0000 | |||
314 | +++ doc/en/release-notes/bzr-2.5.txt 2011-08-16 07:35:26 +0000 | |||
315 | @@ -69,6 +69,9 @@ | |||
316 | 69 | while --match-message, --match-author, --match-committer and | 69 | while --match-message, --match-author, --match-committer and |
317 | 70 | --match-bugs match each of those fields. | 70 | --match-bugs match each of those fields. |
318 | 71 | 71 | ||
319 | 72 | * ``bzr help configuration/<option>`` display the help for ``option`` for | ||
320 | 73 | all registered configuration options. (Vincent Ladeuil, #747050) | ||
321 | 74 | |||
322 | 72 | * Relative local paths can now be specified in URL syntax by using the | 75 | * Relative local paths can now be specified in URL syntax by using the |
323 | 73 | "file:" prefix. (Jelmer Vernooij) | 76 | "file:" prefix. (Jelmer Vernooij) |
324 | 74 | 77 |
TestConfigOptio ndIndex seems like it has an extra 'd' :)
TestConfigOptio nHelp seems unused.