[Kimchi-devel] [PATCH V2] Detect and enable help pages for tabs and plugin tabs

Aline Manera alinefm at linux.vnet.ibm.com
Tue Sep 23 20:44:53 UTC 2014


On 09/22/2014 03:04 PM, Rodrigo Trujillo wrote:
> On 08/28/2014 01:25 PM, Aline Manera wrote:
>>
>> On 08/27/2014 07:18 PM, Rodrigo Trujillo wrote:
>>> Kimchi is currently not able to configure and open help pages of any
>>> plugin tab. This patch fixes this problem and removes the HELP button
>>> if any plugin tab or tab do not have a help configured properly for
>>> the language being used.
>>> (eg. plugins/<tab_html>/help/<lang>/<tab_html>.html does not exist).
>>>
>>> Help pages for a plugin tab follow same Kimchi system:
>>>     - html help file should have the same name of plugin html tab file
>>>     - html files should be in plugin's " ui/pages/help/<LANG> " path
>>>     - plugin should add following lines to <PLUGIN_NAME>.conf:
>>>          * [/help]
>>>          * tools.staticdir.on = True
>>>          * tools.nocache.on = True
>>>
>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>> ---
>>>   plugins/sample/sample.conf.in               |  4 ++++
>>>   plugins/sample/ui/pages/help/en_US/tab.html |  1 +
>>>   src/kimchi/config.py.in                     |  1 +
>>>   src/kimchi/utils.py                         |  9 ++++++--
>>>   ui/js/src/kimchi.main.js                    | 36 
>>> ++++++++++++++++++++++++++++-
>>>   5 files changed, 48 insertions(+), 3 deletions(-)
>>>   create mode 100644 plugins/sample/ui/pages/help/en_US/tab.html
>>>
>>> diff --git a/plugins/sample/sample.conf.in 
>>> b/plugins/sample/sample.conf.in
>>> index 6e0908b..181616d 100644
>>> --- a/plugins/sample/sample.conf.in
>>> +++ b/plugins/sample/sample.conf.in
>>> @@ -22,3 +22,7 @@ tools.kimchiauth.admin_methods = ['POST', 'PUT']
>>>   [/circles]
>>>   tools.kimchiauth.on = True
>>>   tools.kimchiauth.admin_methods = ['POST', 'PUT']
>>> +
>>> +[/help]
>>> +tools.staticdir.on = True
>>> +tools.nocache.on = True
>>> diff --git a/plugins/sample/ui/pages/help/en_US/tab.html 
>>> b/plugins/sample/ui/pages/help/en_US/tab.html
>>> new file mode 100644
>>> index 0000000..cd32b47
>>> --- /dev/null
>>> +++ b/plugins/sample/ui/pages/help/en_US/tab.html
>>> @@ -0,0 +1 @@
>>> +Help page for Kimchi's Sample plugin.
>>> diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
>>> index fca32ee..e10f1f6 100644
>>> --- a/src/kimchi/config.py.in
>>> +++ b/src/kimchi/config.py.in
>>> @@ -145,6 +145,7 @@ class PluginPaths(Paths):
>>>           self.ui_dir = 
>>> self.add_prefix(os.path.join(self.plugin_dir, 'ui'))
>>>           self.mo_dir = 
>>> self.add_prefix(os.path.join(self.plugin_dir, 'mo'))
>>>           self.conf_file = os.path.join(self.conf_dir, '%s.conf' % 
>>> name)
>>> +        self.help_dir = os.path.join(self.ui_dir + '/pages/help')
>>>
>>>
>>>   class UIConfig(dict):
>>> diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
>>> index 0977b9f..9e80e06 100644
>>> --- a/src/kimchi/utils.py
>>> +++ b/src/kimchi/utils.py
>>> @@ -82,13 +82,18 @@ def is_digit(value):
>>>
>>>
>>>   def _load_plugin_conf(name):
>>> -    plugin_conf = PluginPaths(name).conf_file
>>> +    plugin_paths = PluginPaths(name)
>>> +    plugin_conf = plugin_paths.conf_file
>>>       if not os.path.exists(plugin_conf):
>>>           cherrypy.log.error_log.error("Plugin configuration file %s"
>>>                                        " doesn't exist." % plugin_conf)
>>>           return
>>>       try:

>>> -        return Parser().dict_from_file(plugin_conf)
>>> +        plugin_dict = Parser().dict_from_file(plugin_conf)
>>> +        if (os.path.exists(plugin_paths.help_dir) and '/help' in 
>>> plugin_dict):
>>> +            plugin_dict['/help']['tools.staticdir.dir'] = \
>>> +                plugin_paths.help_dir
>>> +        return plugin_dict

Just stopped to check the patch again and the above code should be removed.
It needs to go to plugin config file in [/help] section

>>>       except ValueError as e:
>>>           cherrypy.log.error_log.error("Failed to load plugin "
>>>                                        "conf from %s: %s" %
>>> diff --git a/ui/js/src/kimchi.main.js b/ui/js/src/kimchi.main.js
>>> index ba54b26..f7931da 100644
>>> --- a/ui/js/src/kimchi.main.js
>>> +++ b/ui/js/src/kimchi.main.js
>>> @@ -34,9 +34,10 @@ kimchi.main = function() {
>>>               var path = tab['path'];
>>>               var mode = tab['mode'];
>>>               if (mode != 'none') {
>>> +                var disableHelp = checkHelpFile(path);
>>>                   tabsHtml.push(
>>>                       '<li>',
>>> -                        '<a class="item" href="', path, '">',
>>> +                        '<a class="item', disableHelp,'" href="', 
>>> path, '">',
>>>                               title,
>>>                           '</a>',
>>>                       '</li>'
>>> @@ -46,6 +47,25 @@ kimchi.main = function() {
>>>           return tabsHtml.join('');
>>>       };
>>
>>
>>> +    var checkHelpFile = function(path) {
>>> +        var returnClass;
>>> +        var lang = kimchi.lang.get();
>>> +        url = path.replace("tabs", "/help/" + lang);
>>> +        if (url == "/help" + lang)
>>> +            url = url + "/index.html"
>>> +        else if (/^plugin/.test(url)) {
>>
>> It should be "plugins", right?
>
> Not actually, because it does not matter.  The only difference is that 
> plugin*S* would be more accurate. Not problem to change.

Please, do! Otherwise if we create a URL /plugin it will have an 
unexpected result.
The /plugins is reserved for plugins

>
> The 'path' argument comes as:
>
>     * tabs/host.html        ->    url = /help/en_US/host.html
> * plugins/sample/tab.html       -> url = 
> plugins/sample/help/en_US/tab.html
>
>>
>>> +            var plugin = url.split("/").pop();
>>> +            url = url.replace(plugin, "help/" + lang + "/" + plugin);
>>
>> url = url.replace(plugin, "help/" + lang + "/" + plugin*+ ".html"*);
>
> No, as explained above.

OK.

>
>>
>>
>>> +        }
>>
>> This whole block is the same on kimchi.getHelp() function.
>>
>> Let's create a getHelpURL() and reuse it when needed.
>
> No sure, they look like similar, but are different:
> * checkHelpFile runs in very first kimchi html load time
> * checkHelpFile receives the complete tab html path (such as 
> "/tabs/host.html" )
> * checkHelpFile modifies the path to find the html help file
>
> ** getHelp is used when use clicks in the help button, "runs on-the-fly"
> ** getHelp is receives uses the window path (such as "/#tabs/host")
> ** getHelp open the help page

 From your patch I have:

+    var checkHelpFile = function(path) {
+        var returnClass;
+        var lang = kimchi.lang.get();
+        url = path.replace("tabs", "/help/" + lang);
+        if (url == "/help" + lang)
+            url = url + "/index.html"
+        else if (/^plugin/.test(url)) {
+            var plugin = url.split("/").pop();
+            url = url.replace(plugin, "help/" + lang + "/" + plugin);
+        }


And on getHelp() I have (include current code + your changes)

         var lang = kimchi.lang.get();
         url = url.replace("#tabs", "/help/" + lang);
         if (url == "/help" + lang)
             url = url + "/index.html"

+        else if (/^#plugin/.test(url)) {
+            var plugin = url.split("/").pop();
+            url = url.replace("#", "");
+            url = url.replace(plugin, "help/" + lang + "/" + plugin + ".html");
+        }


else
             url = url + ".html";


Seems to me the code are the same but duplicated in 2 different functions =)

>
> I thought to merge functions at first time but would have to use lots 
> of 'ifs' and 'elses'
> creating a dirty function. Its feasible, but we will have a dirty 
> function that do 2 different
> things. What do you think ?
>
>>> +        $.ajax({
>>> +            url: url,
>>> +            async: false,
>>> +            error: function() { returnClass = " disableHelp"; },
>>> +            success: function() { returnClass = ""; }
>>> +        });
>>> +        return returnClass;
>>> +    };
>>> +
>>>       var parseTabs = function(xmlData) {
>>>           var tabs = [];
>>>           $(xmlData).find('tab').each(function() {
>>> @@ -158,6 +178,15 @@ kimchi.main = function() {
>>>           $(tab).addClass('current');
>>>           $(tab).focus();
>>
>>> +        if ($(tab).hasClass("disableHelp")) {
>>> +            $('#btn-help').prop('disabled', true);
>>> +            $('#btn-help').hide();
>>> +        }
>>> +        else {
>>> +            $('#btn-help').prop('disabled', false);
>>> +            $('#btn-help').show();
>>> +        }
>>> +
>>
>> Is the "else" statement really needed? Because it is the default 
>> button behavior
>
> Yes, it is, because the button is static and the tabs are loaded and 
> change the button dynamically.
> Without the "else", if you move to a  tab without Help and back to a 
> tab with Help, the button will
> never be showed again.

OK.

>
>>
>>
>>>           // Load page content.
>>>           loadPage(url);
>>>       };
>>> @@ -297,6 +326,11 @@ kimchi.getHelp = function(e) {
>>>           url = url.replace("#tabs", "/help/" + lang);
>>>           if (url == "/help" + lang)
>>>               url = url + "/index.html"
>>> +        else if (/^#plugin/.test(url)) {
>>> +            var plugin = url.split("/").pop();
>>> +            url = url.replace("#", "");
>>> +            url = url.replace(plugin, "help/" + lang + "/" + plugin 
>>> + ".html");
>>> +        }
>>>           else
>>>               url = url + ".html";
>>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140923/5879ad01/attachment.html>


More information about the Kimchi-devel mailing list