Room for improvement on my Python code?

Question

I'm working on a Gedit plugin using Python (and PyGTK) and I really havent' worked with Python much so I have no idea if I'm writing Pythonic code.

All of my own code is contained in __init__.py. There are a few other files, but they're from an outside library that I'm hooking into. My __init__.py is as follows:

#
# @file __init__.py
# Does the heavy lifting behind connecting Zen Coding to Gedit.
#

import gedit, gobject, string, gtk, re, zen_core

class ZenCodingPlugin(gedit.Plugin):
    """
    A Gedit plugin to implement Zen Coding's HTML and CSS shorthand expander.

    This file adds the menu items and keyboard shortcuts to the UI and connects
    those items with the good stuff (i.e., the code expansion).
    """

    def __init__(self):
        gedit.Plugin.__init__(self)

    def activate(self, window):
        "Gedit callback: install the expansion feature into the UI"

        ui_manager = window.get_ui_manager()
        action_group = gtk.ActionGroup("GeditZenCodingPluginActions")

        # Create the GTK action to be used to connect the key combo
        # to the Zen Coding expansion (i.e., the good stuff).
        complete_action = gtk.Action(name="ZenCodingAction",
                                     label="Expand Zen code...",
                                     tooltip="Expand Zen Code in document to raw HTML",
                                     stock_id=gtk.STOCK_GO_FORWARD)

        # Connect the newly created action with key combo
        complete_action.connect("activate",
                                lambda a: self.expand_zencode(window))
        action_group.add_action_with_accel(complete_action,
                                           "<Ctrl><Shift>E")

        ui_manager.insert_action_group(action_group, 0)

        # @TODO: Figure out what these lines do
        ui_merge_id = ui_manager.new_merge_id()
        ui_manager.add_ui(ui_merge_id,
                          "/MenuBar/EditMenu/EditOps_5",
                          "ZenCoding",
                          "ZenCodingAction",
                          gtk.UI_MANAGER_MENUITEM, False)
        ui_manager.__ui_data__ = (action_group, ui_merge_id)

    def deactivate(self, window):
        "Gedit callback: get rid of the expansion feature"

        ui_manager = window.get_ui_manager()
        (action_group, ui_merge_id) = ui_manager.__ui_data__

        # Remove the UI data, action group, and UI itself from Gedit
        del ui_manager.__ui_data__
        ui_manager.remove_action_group(action_group)
        ui_manager.remove_ui(ui_merge_id)


    def expand_zencode(self, window):
        "The action which handles the code expansion itself."

        view = window.get_active_view()
        buffer = view.get_buffer()

        # Grab the current cursor position.
        cursor_iter = buffer.get_iter_at_mark(buffer.get_insert())

        # Grab the first character in the line.
        line_iter = cursor_iter.copy()
        line_iter.set_line_offset(0)

        # Grab the text from the start of the line to the cursor.
        line = buffer.get_text(line_iter, cursor_iter)

        # Find the last space in the line and remove it, setting a variable
        # 'before' to the current line.
        words = line.split(" ")
        before = words[-1].lstrip()
        if not before:
            return

        # Get the language of the current document. Second line prevents an error
        # if first line returns None.
        lang = window.get_active_document().get_language()
        lang = lang and lang.get_name()

        # Using the 'before' variable, convert it from Zen Code
        # to expanded code. If there isn't anything, just return.
        if lang == 'CSS':
            after = zen_core.expand_abbreviation(before,'css','xhtml')
        else:
            after = zen_core.expand_abbreviation(before,'html','xhtml')
        if not after:
            return

        # Grab the line's indentation and store it.
        indent = re.match(r"\s*", line).group()

        # Automatically indent the string and replace \t (tab) with the
        # correct number of spaces.
        after = zen_core.pad_string(after,indent)
        if view.get_insert_spaces_instead_of_tabs():
            tabsize = view.get_tab_width()
            spaces = " " * tabsize
            after = after.replace("\t",spaces)

        # We are currently lame and do not know how to do placeholders.
        # So remove all | characters from after.
        after = after.replace("|", "")

        # Delete the last word in the line (i.e., the 'before' text, aka the
        # Zen un-expanded code), so that we can replace it.
        word_iter = cursor_iter.copy()
        position_in_line = cursor_iter.get_line_index() - len(before)
        word_iter.set_line_index(position_in_line)
        buffer.delete(word_iter, cursor_iter)

        # Insert the new expanded text.
        buffer.insert_at_cursor(after)

I'm just asking because the above doesn't seem very object oriented and it kind of strikes me as a bad idea to put this much logic in __init__.py, but being new at this, I'm not sure.

Is there room for improvement? If so, how?

(I'm trying to shy away from what the plugin actually does because I'm looking more for a coding style review than a logarithm review, but if you need to see the code from the outside library, the whole plugin is at here)

Answer 1

Perhaps you want to move it into a file called zen_plugin.py

Then make your __init__.py

from zen_plugin import ZenCodingPlugin

Answer 2

I've never done a GEdit plugin, so i can't comment on the _init_.py issue, but general notes:

• Isn't calling parent's _init_ with no new args redundant -- can't you just take out those 2 lines?

• You create more local variables than I would have. I had to keep looking around to see where a value came from, or if it's used again. For example:

  tabsize = view.get_tab_width()
  spaces = " " * tabsize
  after = after.replace("\t",spaces)
  

  could be:

  after = after.replace("\t", " " * view.get_tab_width())
  

• A little redundancy here:

  if lang == 'CSS':
      after = zen_core.expand_abbreviation(before,'css','xhtml')
  else:
      after = zen_core.expand_abbreviation(before,'html','xhtml')
  

  compare:

  after = zen_core.expand_abbreviation(before, 'css' if lang == 'CSS' else 'html', 'xhtml')
  

Other than that, it looks like pretty decent Python code to my eye.

Answer 3

I would try to extract some functions from the long expand_zencode(). Like an expand_tabs(), for example. That's a matter of taste to some extent, but whenever I see a 'travelogue' with comments pointing out 'sights' along the way, it's a strong hint to refactor into functions each with a doc comment instead. (Not necessarily one-to-one; I don't have the energy/knowledge for detailed advice on this function.) This change automatically addresses Ken's complaint about keeping track of the local variables.

BTW, you have an odd definition of tab-expansion: changing each tab character to tabsize spaces. I suppose there's a reason.

This is unneeded:

def __init__(self):
    gedit.Plugin.__init__(self)

The code for the before variable doesn't seem to match its comment. (And the .lstrip() is redundant, isn't it?)

You sometimes have spaces between arguments and sometimes not; IIRC the Python style guide wants you to consistently use spaces. (foo(x, y) and not foo(x,y).)

Your class's comment says: "This file adds...". Shouldn't that be "This plugin adds..."?

Consider asking this sort of thing on http://refactormycode.com/ instead.

Answer 4

It's gui code and is always verbose, my best advice is: does it work? good. Next Task !

Answer 5

If you only have the one file then your directory structure probably looks like

pluginname
`- __init__.py

In this case you can just flatten your project to

pluginname.py

If you later need other modules in your package you could go to

pluginname
+- __init__.py
`- plugin.py

where plugin.py has this class in it and in __init__.py put

from pluginname.plugin import ZenCodingPlugin

This way anything that was previously using from pluginname import ZenCodingPlugin or import pluginname; pluginname.ZenCodingPlugin would still work.