Re: [kupfer] [PATCH] plugin.google: Add another searchplugins dir



Oh I think I got it. We add all the plugins directory, then 
we handle IO and OS exceptions in the for loop later.

Do you want me to submit a new patch or you'll fix it yourself? I prefer the latter.

--
Francesco

2009/9/28 Ulrik Sverdrup <ulrik sverdrup gmail com>
Hi,

Thanks for good patches! Comments below

2009/9/28 Francesco Marella <francesco marella gmail com>:
> From 69584de355aa89b58660fda6da30ae077cd28803 Mon Sep 17 00:00:00 2001
> From: Francesco Marella <francesco marella gmail com>
> Date: Mon, 28 Sep 2009 02:45:06 +0200
> Subject: [PATCH] plugin.google: Add another searchplugins dir
> ---
>  kupfer/plugin/google.py |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> diff --git a/kupfer/plugin/google.py b/kupfer/plugin/google.py
> index ead8e1d..e934563 100644
> --- a/kupfer/plugin/google.py
> +++ b/kupfer/plugin/google.py
> @@ -169,6 +169,8 @@ class OpenSearchSource (Source):
>   plugin_dirs.extend(config.get_data_dirs("searchplugins",
>   package="iceweasel"))
>
> + plugin_dirs.append("/usr/lib/firefox-addons/searchplugins")
> +
>   self.output_debug("Found following searchplugins directories",
>   sep="\n", *plugin_dirs)
>
I think that we should check existence right here, since the other
calls do that (implicitly); then we don't have to double-do that work
later in the for loop. Something like:

       addons = "/usr/lib/firefox-addons/searchplugins"
       if os.path.exists(addons):
           plugin_dirs.append(addons)

I have seen the second patch, but I think it would probably be a good
idea to have error checking per-directory in the loop. However, this
should be done with exceptions; with exceptions, we defend against
errors we don't anticipate; neither my old version nor the new with
existance check guards against other OS errors. (Existing but
forbidden to read directory?). I just checked, and Kupfer fails in
this scenario!

Ulrik



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]