Hi!
   I made some changes and in the case:

+        textbox_text=self.textbox.get_text()
+        self.chooser.set_current_name(os.path.basename(textbox_text))
+        self.chooser.set_current_folder(os.path.dirname(textbox_text))

You are missing spaces around '=' (It should be an error from pylint or
PEP8)

The same thing in
+    GLADE_FILE = os.path.join(current_rep, "taskbrowser.glade")
+    MODIFYTAGS_GLADE_FILE= os.path.join(current_rep,
"modifytags_dialog.glade")
+    TAGEDITOR_GLADE_FILE = os.path.join(current_rep, "tageditor.glade")

   I checked and lint and pep8 didnot give any error when I removed spaces
between operator and the operands

and also,
   in case of unused variables. I have simply commented them.

Nimit Shah,
B Tech 3rd year,
Computer Engineering Department,
SVNIT Surat
Secretary ACM-NIT Surat
www.dude-says.blogspot.com



On Sat, Jun 23, 2012 at 6:23 PM, Izidor Matušov <email address hidden>wrote:

> Review: Needs Fixing code
>
> 156     + _("Do you really want to remove the '%s' \
> 157     + synchronization service?") % \
>
> You can't break the line inside a string like that. It will add white
> space inside the string. The better way to solve it would be:
>
> message_format = _(
> "Do you really want to remove the '%s' synchronization service?"
> ) % backend.get_human_name())
>
> (You can shift the string more to left so you don't have too long line).
>
> anoter possibility is to split string like
>
> _("Do you really want to remove the '%s' "
> "synchronization service?") % \
>
>
> ***************************
>
>                 markup = ngettext("Syncing a task tagged %s" % tags_txt, \
>                     "Syncing %d tasks tagged %s" % (tasks_number,
> tags_txt), \
> -                              tasks_number)
> +                              tasks_number)
>
> Delimiters \ are not needed after a comma in parameters. You can ommit
> them :)
>
>
> *****************************
>
> +        textbox_text=self.textbox.get_text()
> +        self.chooser.set_current_name(os.path.basename(textbox_text))
> +        self.chooser.set_current_folder(os.path.dirname(textbox_text))
>
> You are missing spaces around '=' (It should be an error from pylint or
> PEP8)
>
> The same thing in
> +    GLADE_FILE = os.path.join(current_rep, "taskbrowser.glade")
> +    MODIFYTAGS_GLADE_FILE= os.path.join(current_rep,
> "modifytags_dialog.glade")
> +    TAGEDITOR_GLADE_FILE = os.path.join(current_rep, "tageditor.glade")
>
>
> ********************************
>
>         tasks = self.req.get_tasks_tree()
> -        for event in 'node-added-inview', 'node-modified-inview',
> 'node-deleted-inview':
> +        l =['node-added-inview', 'node-modified-inview',
> 'node-deleted-inview']
> +        for event in l:
>             handle = tasks.register_cllbck(event, self._update_tags)
>             self.tag_cllbcks.append((event, handle))
>
>
> It is better to split the list rather than put it into an artificial
> variable:
>
>  for event in ['node-added-inview', 'node-modified-inview',
>               'node-deleted-inview']:
>
> *********************************
>
> -        while (it.get_offset() < end.get_offset()) and (it.get_char() !=
> '\0'):
> +        while(it.get_offset() < end.get_offset()) and (it.get_char() !=
> '\0'):
>
> Parenthesis could be ommitted for better readability:
> while it.get_offset() < end.get_offset()) and (it.get_char() != '\0':
>
> (it is there about 3 times or so)
>
> ************************************
>
>                     if url.startswith("bug #") or url.startswith("lp #"):
>                         topoint = "https://launchpad.net/bugs/%s" %nbr
>                     elif url.startswith("bgo #"):
> -                        topoint = "
> http://bugzilla.gnome.org/show_bug.cgi?id=%s" %nbr
> +                        topoint = "http://bugzilla.gnome.org/show_bug.cgi
> ?\
> +                                  id=%s" %nbr
>                     elif url.startswith("bko #"):
> -                        topoint = "
> https://bugs.kde.org/show_bug.cgi?id=%s" %nbr
> +                        topoint = "
> https://bugs.kde.org/show_bug.cgi?id=%s"\
> +                                   %nbr
>                     elif url.startswith("fdo #"):
> -                        topoint = "
> http://bugs.freedesktop.org/show_bug.cgi?id=%s" %nbr
> +                        topoint = "
> http://bugs.freedesktop.org/show_bug.cgi?\
> +                                   id=%s" %nbr
>
> +    Log.info("GNOME keyring was not found, passwords will be not \
> +                                     stored after restart of GTG")
>
>
>
> Also bad splitting of strings...
>
> ******************************
>
> There are alsow those "errors" in files you touched:
>
> GTG/tools/cleanxml.py:24: 'time' imported but unused
> GTG/gtk/backends_dialog/addpanel.py:176: local variable 'label' is
> assigned to but never used
> GTG/gtk/backends_dialog/__init__.py:253: local variable 'backend_id' is
> assigned to but never used
> GTG/gtk/browser/treeview_factory.py:32: 'Date' imported but unused
> GTG/gtk/editor/taskview.py:514: local variable 'tags_before' is assigned
> to but never used
> GTG/gtk/editor/taskview.py:821: local variable 'selec' is assigned to but
> never used
>
> (you need pyflakes to see them)
>
> You havent't cleanup errors from pylint yet (they are much more
> challenging), but it is okay to do them in another merge request.
>
> Please, fix the errors stated above, I'll check it again and you will
> probably get approve from me :)
> --
> https://code.launchpad.net/~nimit-svnit/gtg/cleanup/+merge/111709
> You are the owner of lp:~nimit-svnit/gtg/cleanup.
>