[Skencil-devel] Things to fix for Skencil 1.0

Bernhard Herzog bh at intevation.de
Thu Dec 9 16:29:16 CET 2010


Hi Igor,

On 26.11.2010, Igor Novikov wrote:
>> 5. skconvert and skshow are missing
>>
>> They should be reactivated again.  Both require that the main Sketch can
>> be imported without running the full Skencil application, which is
>> impossible right now without changes.  Also, skconvert must work without
>> an X-connection as mentioned above.
>>
> We have discussed this issue inside sK1 Team and decided that there is
> no reason to support skconvert because a lot of filters are out of
> date. We would propose using UniConvertor as a translation tool
> because this application has a good *.sk format support and allows
> processing more formats than Skencil/skconvert supports.

skconvert, as well as skshow and sk2ppm which I forgot to mention, are not
only useful to Skencil users -- sk2ppm in particular is used by the scripts
that generate the gallery section of the web-pages -- they also demonstrate
how to use Skencil as a library.  They are worth keeping around.  They may
not have to be installed by default, but there is no reason to remove them
from the sources.

> Also UniConvertor doesn't depend on Xlib and has been ported on win32
> and macosx.

skconvert doesn't depend on Xlib either.

> > 7. Formatting:
> >
> > Some files have very long lines and much trailing whitespace:
> >
> >   src/Sketch/UI/ruler.py
> >   src/Sketch/Base/gtkutils.py
> >   setup.py

> The lines are not wrapped because in such form the code is just easy
> for reviewing.  Wrapping was important 5-10 years ago when 15-17"
> monitors were conventional. But now 22-24" displays are widely used as
> developer's and user's hardware:

Small monitors aren't the only reason to keep lines reasonably short.
Some other reasons:

 - The existing code fits into 80 columns in almost all places.

 - Long lines often make reading more difficult.

 - Short lines allow me to two editor windows open side-by side.

 - Long lines are often a code-smell.  It can indicate that the nesting
   is too deep or identifiers are too long, or in this case code duplication.

 - Usually there are many short lines and relatively few long lines in a
   file.  This leads to lots of wasted space.

 - Short lines help when code fragments are embedded in other texts such as 
   documentation or emails.

On the whole, I see no reason why lines longer than 80 columns should be 
allowed in Skencil, except in places where it's really hard to keep the lines 
shorter, which should rarely be the case.

> http://saveimg.ru/pictures/23-11-10/1692954d8b3e745b9e55e45fff85c1f0.png

Even if the lines in that example were short enough, that wouldn't be an
example of well-written code with all the duplication.  Only two of the
arguments of widget.tk.call change from line to line, yet all the others
are repeated over and over again.  Use a loop or a wrapper function.  That
will automatically make the lines short enough.  For example:

    def set_option(key, value):
        widget.tk.call('option', 'add', key, value, 'interactive')

    set_option('*background', color_scheme.bg)
    set_option('*foreground', color_scheme.fg)
    set_option('*selectForeground', color_scheme.selectforeground)
    set_option('*selectBackground', color_scheme.selectbackground)
    # ...

Of course, there's still some duplication.  One should perhaps also
try to remove the duplication of the "color_scheme." part.

  Bernhard


More information about the Skencil-devel mailing list