Skip to content

Critique #1

@Metallicow

Description

@Metallicow

For a first-pass at the code, I would like to say you have done a good job. :)

There are a few things that don't quite work as I would expect.

  1. Dragging on the minimap and moving the mouse outside the minimap doesnt scroll properly like I've seen other implementations of a minimap do.(either dragging up or down the document)

  2. This should be changed self.SetDoubleBuffered(True)
    to

if not self.IsDoubleBuffered():
    self.SetDoubleBuffered(True)

Robin states that some platforms are already DoubleBuffered by default, and doing it without a check would x4 or in otherwords make performance worse.

  1. I see double loops often enough that a local opt list comprehension will definitely run faster since the method is pushing into C when dealing with all pixels.
    def SetTransparency(self, alpha=0x80):
        img = self.bmp.ConvertToImage()
        if not img.HasAlpha():
            img.InitAlpha()
            for x in range(img.Width):
                for y in range(img.Height):
                    img.SetAlpha(x, y, alpha)
            self.bmp = img.ConvertToBitmap()

would run faster as

    def SetTransparency(self, alpha=0x80):
        img = self.bmp.ConvertToImage()
        if not img.HasAlpha():
            img.InitAlpha()
        width, height = img.Width, img.Height
        SetAlpha = img.SetAlpha
        [SetAlpha(x, y, alpha)
            for y in range(height)
                for x in range(width)]
        self.bmp = img.ConvertToBitmap()
  1. As far as the #INFO, Class with too many parameters: better design strategy?, I personally will put all that stuff with the defaults in the class constructor, so it shows up when you request a calltip, but am not sure if that really should be written another way. I would assume most folks would only use 1 minimap, but I wouldnt rule out someone using it with dynamic sash or multisash type of setup.

I think that's all I see at the moment. Thanks for spending the time to code this thing up! I always had it on my list of widgets to implement, but never seemed to get the time to code one up.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions