← Back to team overview

kicad-developers team mailing list archive

Re: [FEATURE] Component table viewer

 

Oliver,

I finally got a chance to test your patch set and was a bit surprised
what I saw after following the conversation on the mailing list.  I was
under the impression that this was a generic component properties
editing grid not a BOM tool which is what it really is.  I like the idea
of being able to edit component fields in table form.  I'm less thrilled
about the BOM export options.  For those of you who haven't been around
very long, Eeschema used to have a BOM dialog.  It didn't allow for
editing field values but it contained options for various BOM output
types.  Initially this dialog was simple and contained only a few BOM
output types and options.  Of course everyone has their own idea of how
a BOM should be formatted so gradually over time, the BOM dialog and the
underlying BOM output code became a huge mess.  It was finally decided
that the design was no longer maintainable and removed.  It was replaced
by the current system along with samples that provided all of the same
BOM output options from the old BOM dialog.  Except for the field
editing grid, your dialog and BOM code looks a lot like the original BOM
dialog.  I can see the same thing happening all over again.  Why no use
the existing BOM generation code in your dialog rather than re-implement
code that does the exact same thing?  I'm not opposed to field editing
part of the dialog, but I see the BOM output part heading the same
direction as the old BOM dialog.

On 4/20/2017 1:59 AM, Oliver Walters wrote:
> Wayne,
> 
> Is the behaviour I have implemented acceptable?
> 
> Regards,
> Oliver
> 
> On Wed, Apr 19, 2017 at 12:13 AM, Oliver Walters
> <oliver.henry.walters@xxxxxxxxx <mailto:oliver.henry.walters@xxxxxxxxx>>
> wrote:
> 
>     Wayne,
> 
>     I have now fixed this such that UNDO actions are pushed to the UNDO
>     stack for the associated sheet. All UNDO actions for a given sheet
>     are grouped so a single Ctrl-Z will undo all components changed in
>     the table (for the given sheet).
> 
>     Please find patch _007 attached (must be appli ed atop all previous
>     patches).
> 
>     Let me know if you see any other pressing issues.
> 
>     Regards,
>     Oliver
> 
>     On Tue, Apr 18, 2017 at 6:30 AM, Wayne Stambaugh
>     <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>> wrote:
> 
>         On 4/17/2017 4:18 PM, Oliver Walters wrote:
>         > So how do we proceed here? Is there a 'global' undo stack? If not:
> 
>         Unfortunately there is no global undo stack.  Undo stacks are
>         maintained
>         for each unique SCH_SCREEN (schematic file) object.
> 
>         >
>         > A) don't allow changes made in the component table viewer to be undone
>         > B) Make an undo entry for each sheet that has changed symbols
>         >
>         > A) is easier but the user would need to quit-without-save to undo changes
> 
>         This is less than desirable
> 
>         >
>         > B) is more difficult and doesn't solve the undo operations getting out
>         > of order either, as the user could inject another operation on a given
>         > sheet.
> 
>         This would be my preference.  Out of order operations are already an
>         issue so this solution doesn't make that issue any worse. 
>         Undo/redo is
>         only available for the current sheet so the user would have to
>         change
>         sheets in order to undo anything changed in the component
>         properties table.
> 
>         >
>         > Suggestions?
>         >
>         > On 18 Apr 2017 01:26, "Wayne Stambaugh" <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
>         > <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>> wrote:
>         >
>         >     On 4/17/2017 10:21 AM, jp charras wrote:
>         >     > Le 17/04/2017 à 04:11, Oliver Walters a écrit :
>         >     >> JP, others,
>         >     >>
>         >     >> After further investigation, I have worked out why the components
>         >     with duplicated references were
>         >     >> displaying incorrectly.
>         >     >>
>         >     >> Patch_004 is attached, Thomas can you confirm that it fixes the
>         >     display for you?
>         >     >>
>         >     >> Kind Regards,
>         >     >> Oliver
>         >     >>
>         >     >> On Mon, Apr 17, 2017 at 7:53 AM, Oliver Walters
>         >     <oliver.henry.walters@xxxxxxxxx
>         <mailto:oliver.henry.walters@xxxxxxxxx>
>         <mailto:oliver.henry.walters@xxxxxxxxx
>         <mailto:oliver.henry.walters@xxxxxxxxx>>
>         >     >
>         >     > Good work, Oliver!
>         >     >
>         >     > I found 2 issues (tested on W7)
>         >     >
>         >     > 1 - m_reloadTableButton is not correctly enabled/disabled.
>         >     > This is due to the way events are managed, and this is
>         OS dependent.
>         >     > To avoid this issue, enable/disable it inside a
>         wxUpdateUIEvent
>         >     attached to this button.
>         >     >
>         >     > 2 - ESC key and ENTER keys do not dismiss the dialog.
>         >     > This is due to the fact you do not have a
>         wxStdDialogButtonSizer,
>         >     and no OK and Cancel button.
>         >     > Please, add it and use the OK button (as usual in a
>         dialog) to
>         >     transfer changes to schematic (do not
>         >     > use a wxCloseEvent to manage that), and obviously Cancel
>         just
>         >     closes the dialog.
>         >     > To do this transfer, just  override
>         TransferDataFromWindow(), that
>         >     is called by wxWidgets when
>         >     > closing a dialog by the OK button.
>         >     >
>         >     > About other things, undo/redo lists should manage only
>         changes
>         >     made inside the corresponding sheet,
>         >     > not in other sheets, to avoid inconsistencies and
>         therefore crashes.
>         >     >
>         >
>         >     This is one of the reasons I've been reluctant to accept
>         code that
>         >     attempts to change the state of a SCH_SCREEN object other
>         than the
>         >     current SCH_SCREEN object.  It exposes a known flaw in our
>         schematic
>         >     undo/redo design and I have yet to see anyone update the
>         undo/redo
>         >     SCH_SCREEN stacks correctly.  I see the potential for
>         serious issues if
>         >     you do not keep the undo/redo stacks properly synced. 
>         Once you allow
>         >     the modification of information in the SCH_SCREEN object
>         other than the
>         >     current one, you need to update the undo/redo stack for
>         the appropriate
>         >     SCH_SCREEN object.  Otherwise, you wont be able to undo
>         all of the
>         >     changes correctly.
>         >
>         >     _______________________________________________
>         >     Mailing list: https://launchpad.net/~kicad-developers
>         <https://launchpad.net/~kicad-developers>
>         >     <https://launchpad.net/~kicad-developers
>         <https://launchpad.net/~kicad-developers>>
>         >     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>         >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>         >     Unsubscribe : https://launchpad.net/~kicad-developers
>         <https://launchpad.net/~kicad-developers>
>         >     <https://launchpad.net/~kicad-developers
>         <https://launchpad.net/~kicad-developers>>
>         >     More help   : https://help.launchpad.net/ListHelp
>         <https://help.launchpad.net/ListHelp>
>         >     <https://help.launchpad.net/ListHelp
>         <https://help.launchpad.net/ListHelp>>
>         >
> 
> 
> 



References