kicad-developers team mailing list archive

[jp charras <jp.charras@xxxxxxxxxx>]

Le 28/04/2013 13:41, Lorenzo Marcantonio a écrit :
> I was comparing my tree with the master one. These are some changes
> I did in times and I would like what do you think of these
>
> - GenDate() at the moment uses a fixed format (more or less the european
>    one), with the months hardcoded to the short english names.
>
>    Wouldn't it be better to use the localised formatter? I'm using
>    "%e %B %Y" but even better would be the strftime %x formatter
>
> - In the grid drawing code there is a special case to show
>    a double-pitch grid before suppressing the drawing. That's confusing
>    if you're using the grid to estimate distances. So if the grid doesn't
>    fit, just hide it

I disagree: a grid with a double pitch is still useful, when you need a 
grid.

To estimate distances there are (by far) better ways than count grid points.

> - Many of the macros in macros.h would be better defined as templated
>    functions (unless there is some weird preference for macrofunctions in
>    C++). In particular EXCHG wouldn't need the boost typeof machinery.
>    It's pretty simple, actually:
>
>    template<class T, class T2> inline void EXCHG( T& a, T2& b )
>    {
>      T temp = a;
>      a = b;
>      b = temp;
>    }
>
>    A similar approach holds for the other ones. If it wasn't for some
>    mixed-type usage (double with int, for angles, IIRC), std::swap would
>    be even better.
Agreed.

> - The same hold for SAFE_DELETE; the current one in particular is *not*
>    protected against accidental single-statement use i.e.
>
>    if (stuff) SAFE_DELETE(x);
>
>    generates the equivalent
>
>    if (stuff) delete x;
>    x = NULL;
>
>    which is clearly wrong; if (for some unknown reason) a macro is
>    preferred at least we should add a do { ... } while (0) guard

As far I remember, it was already pointed out. But the fix was never made.
This is good to fix it.

> - ReturnLayerPair is a strange name for an accessor... I propose to
>    rename it to GetLayerPair (IIRC it already happened int the past)
>
> - I have 'split' the two layer in a via adding a bottomlayer member
>    instead of that nasty hack of bit-encoding them in the primary layer.
>    It only accessor-used but it's ugly (it will come back if someone
>    subclasses the via class, anyway)
>
> - Question: is 7000 a magic number for a layer in specctra_import?
>    a define or at least a comment would be useful.
>
> - In the tooltips inside dialog_edit_module_for_Modedit.cpp smd should
>    be uppercased :D
>
> Other layer stuff is in standby until we decide what to do (i.e.
> probably until we have the new library format working)


--
Jean-Pierre CHARRAS