kicad-developers team mailing list archive

[jean-pierre charras <jean-pierre.charras@...>]

Dick Hollenbeck a écrit :
> Wayne Stambaugh wrote:
> > Dick,
> >
> > I made the change. I apologize for the error. I forgot to check
> > the popup commands before submitting to code. In my zeal to
> > simplify the eeschema "main loop", I missed the default handler in
> > the first case statement. I will make sure to be more careful in
> > the future.
> >
> > I still feel this function needs to be simplified. It is over 700
> > lines of code long. It is also redundant. The wxWidgets command
> > loop does this already. I can understand grouping related commands
> > together in an event handler. Recombining them back into a single
> > function call and picking apart again doesn't make a lot of sense
> > to me but maybe I'm missing the big picture. If you wanted to add
> > a new command (say a block annotate command), you have to decipher
> > this function to make sure you don't break anything. Obviously I
> > deciphered it incorrectly.
> >
> > Wayne
> >
>
> Wayne,
>
> I did not write this code, so I have no insight into why it is
> structured the way it is. Unless someone comes forward soon with a
> good reason to leave it alone, *other than* "if it's not broken then
> don't fix it", then maybe it should be subject to your improvements.
> Jean-Pierre probably wrote it, but he has said clearly that he wants
> things cleaned up and improved.
>
>
> I don't have any strong opinions on it.
>
> Dick

Wayne,

Yes, sometimes, code needs to be cleaned .
The eeschema main loop was started with only few commands to handle (and 
it was a small file.)
But a lot of commands were added , and this file growed.

So, now, his is not unreasonable to simplify it.

wxWidgets works fine but does not solve all problems!

We must consider some things:
There are hundred of possible commands (tools, hot keys, pop up menus...).
So one function per command is equivalent to hundred of functions (and 
most of functions are very short and have only 3 or 4 lines)!
The code maintenance could be difficult.

But also, take care to this:
we could believe a function called by a command must handle one thing: 
what we want to do.

This is false:
any command must handle two things:
1 - what we want to do.
2 - what we are doing.

Let me explain what I mean:
In may cases, one can do a command withing an other command: a zoom 
command when we are moving an item.

Some commands can be (and must be) done withing the current command (a 
zoom command or a rotate component is such an example)
And others *not* :
You are working on a schematic project using a hierarchy.
Imagine what happens when moving a component and the F5 key is activated 
( Next Search) and the search switches on an other sheet !
(Pop Up menus and mainly hot keys are critical from this point of view, 
and for hot keys this issue is the main reason which explain why kicad 
has very few hot keys)

Some current commands *must* be aborted, and others *not*.

As you said, i believe grouping related commands together is a good 
compromise:
Only some commands are grouped, are it is easy to see when a command 
abort the current command or not.
(only few duplicate code in few functions or files)
The file (and the switch structure used) has a reasonnable size.
We do not need to maintain hundred of functions (one per command).
And, of course the "Abord_Current_Command" could be a function if it is 
used in some files or functions.
So if you move commands out of the eeschema ( or pcbnew) main loop 
which handles abort current command , do not forget to call this 
"Abord_Current_Command"
(keep in mind these functions could be called -in the future- by an hot 
key, and some of them are already callable.)

Jean-Pierre CHARRAS