On Sun, 2008-05-25 at 08:57 +0200, Kristian Lyngstøl wrote:
> On Sun, May 25, 2008 at 2:46 AM, Sam Spilsbury <[EMAIL PROTECTED]> wrote:
> > On Sun, May 25, 2008 at 1:30 AM, Kristian Lyngstøl
> > <[EMAIL PROTECTED]> wrote:
> >> 7. +    ms->steps = (float) time / ((20.1 - speed) * 10); <--- This
> >> seems quite random.
> >
> > I'll probably have a add a comment on how that works. It's kind of like
> > a geometric series that stops at a particular point, that is, when time is
> > 0. 20.1 - speed is exactly that, the higher the speed, the lower the 
> > divisor.
> > I'm guessing I could refactor it to divide by some constant and multiply by
> > speed.
> 
> I'll look more closely at this when you've got a comment and during
> testing of actual functionality :)
> 
> >> 8. Coding style is generally messy and/or random.
> >>
> >
> > Can you give an example so I know what to fix? Thanks =)
> 
> I'll try, looking closer, some of this is probably indentation, which
> can be hard to display in a mail reader that doesn't use monospace...
> 
> 154 +/* Animation Prep */
> 155 +static void
> 156 +maximumizePreparePaintScreen (CompScreen *s,
> 157 +
>  int             time)
> 158 +{
> 
> 172 +static Bool
> 173 +maximumizePaintWindow (CompWindow                  *w,
> 174 +                                                     const
> WindowPaintAttri    b   *attrib,
> 175 +                                                     const
> CompTransform               *transform,
> 176 +                                                     Region
>              region,
> 177 +                                                     unsigned int
>                      mask)
> 178 +{
> 179 +    Bool       status;
> 180 +    CompScreen *s = w->screen;
> 181 +    CompTransform mTransform = *transform;
> 
> Generally speaking you shouldn't go over the 76th character. In files
> that grow large, I prefer to keep within 75 myself. (leaving 4 for
> line numbers and a fifth for space), but as long as it's less than 80,
> that's usually acceptable.

OK, I'll rework the line spacing so that it doesn't go over around 78.
I'm
usually used to gedit (Don't shoot me for not using (insert text editor
of
your choice here), I really don't have the time to learn all of vi...),
running
fulllscreen @ 1680x1050, so it's easy to get fooled into writing big
lines.

> 
> 229 +       if (mw->stage != stageNone)
> 230 +       addWindowDamage (w);
> 
> 300 +       md->screenPrivateIndex = allocateScreenPrivateIndex (d);
> 301 +    if (md->screenPrivateIndex < 0)
> 
> ....
> 
> 424      static InitPluginObjectProc dispTab[] = {
> 425 -       (InitPluginObjectProc) 0, /* InitCore */
> 426 -       (InitPluginObjectProc) maximumizeInitDisplay,
> 427 -       0,
> 428 -       0
> 429 +           (InitPluginObjectProc) 0, /* InitCore */
> 430 +           (InitPluginObjectProc) maximumizeInitDisplay,
> 431 +           (InitPluginObjectProc) maximumizeInitScreen,
> 432 +           (InitPluginObjectProc) maximumizeInitWindow
> 433      };
> 
> Here you're changing whitespace from correct to incorrect indentation....
> 
> It was mostly just the same things, I also so a WRAP(foo) UNWRAP(foo)
> (no space between function/macro and opening parentheses)  , but
> looking closer, that was commented out anyway.

Hmm, I'll have a look in my text editor's settings, I have a feeling
something is
wrong...

As for the space between function (arguments) I'll make sure that
happens =)

> 
> - Kristian

I don't really have the time to work on this over the next week due to
assignments
exams taking their toll on my time, but I'll try and submit some new
patches next
week.

- Sam

> _______________________________________________
> Dev mailing list
> [email protected]
> http://lists.compiz-fusion.org/mailman/listinfo/dev

_______________________________________________
Dev mailing list
[email protected]
http://lists.compiz-fusion.org/mailman/listinfo/dev

Reply via email to