Allowing comments after the line continuation backslash

[Lawrence D'Oliveiro]

In message <878w12kt5x.fsf.mdw at metalzone.distorted.org.uk>, Mark Wooding 
wrote:

> Lawrence D'Oliveiro <ldo at geek-central.gen.new_zealand> writes:
> 
>> Maybe you should look at the code in context
>> <https://github.com/ldo/dvd_menu_animator>, then you can express some
>> more opinions on how to improve it.
> 
>   1. class keyval: `are there official symbolic names for these
>      anywhere?'  The gtk.keysyms module contains `Return', `KP_Enter',
>      `Left', `Up', `Right', and `Down'.

Thanks, that’s useful. I couldn’t find mention of such in the documentation 
anywhere.

>      Question: why are `returnkey' and `enterkey' defined in hex and the
>      others in decimal?

Can’t remember now. Does it matter?

>   2. The MainWindow class only has the `Window' attribute described in
>      its definition.  Apparently there are other attributes as well (the
>      ColorMumbleList family for one, also `LayerDisplay').  Similarly,
>      the LoadedImage class has attributes `Contents' and `FileName', but
>      I see attributes `RowStride', `ImageDrawSize' and `SrcDimensions'
>      used immediately below.

These are really just namespaces, one for the GUI context and the other for 
the image data.

>   3. In SelectSaveMenu, there's a curious `for' loop:
> 
>         for State in (gtk.STATE_NORMAL,) : # no need for
>         gtk.STATE_INSENSITIVE
>           ## blah
> 
>      Firstly, this is obviously equivalent to `state = gtk.STATE_NORMAL'
>      followed by the loop body (YAGNI).

It’s a tuple of one element. It used to be a tuple of two, and there is the 
possibility it might need to become that again (as intimated at in the 
attached comment). That’s why it stays a tuple.

>   4. Ahh!  I've tracked down where the MainWindow is actually
>      populated.  Ugh.  The code appears to revel in boilerplate.  (For
>      those at home, we're now looking at `SetupMainWindow', specifically
>      at the calls to `DefineScrollingList'.)  I count six calls, each
>      nearly (but not quite) identical, of about eleven lines each.

Could be worse. Imagine if I had written out the 30 lines of that function 
out each time instead.

>      Of course, being boilerplate, the similarities visually outweigh the
>      differences, when in fact it's the differences that are
>      interesting.

That is generally how code reuse works. All the “boilerplate” is in the 
function definition, so I just have to call it parameterized by the 
differences appropriate to each instance.

>      It doesn't help that all of the names of the things involved are so
>      wretchedly verbose.

Oh please, no. Since when are explanatory names “wretchedly verbose”?

>      How about the following?
> 
>         def make_listview_l1 ...

And what, pray tell, is the significance of the name “make_listview_l1”? If 
there is something I would describe as “wretched”, it is the use of random 
numerical suffixes like this.

>     Looking at some of the rest of the code, it might (or might not) be
>     worthwhile actually making a class to gather together a list's model
>     and view; subclasses can then vary their individual parameters, and
>     the various colour lists can also keep the various captions with
>     them.

Most of that variation is already handled without the limitations of 
thinking in classes. For example, selection of a colour in all three lists 
is handled through a single EditColor routine. And of course you’ve already 
seen how a single DefineScrollingList routine can be used to set up all the 
scrolling lists used in this GUI.

>   5. Much of the indentation and layout is rather unconventional, though
>      not intolerable.  But I found (deep in `SelectSaveMenu'):
> 
>         NewRenderPixels = array.array \
>           (
>                 "B",
>                         FillColor
>                 *
>                         (ImageRowStride // 4 * ImageBounds[1])
>           )
> 
>       to be most disconcerting.

Is the structure of the expression not apparent to you? You’d probably need 
plenty of fresh air after something like this, then:

        PatternArray = array.array \
          (
            "I",
                16 * (16 * (Light,) + 16 * (Dark,))
            +
                16 * (16 * (Dark,) + 16 * (Light,))
          )


>       Also, I couldn't work out why some parens are indented only two
>       spaces and others are indented the full eight.

Eight?? You mean four.

See, this is why I should probably stop using tabs...