Comment 46 for bug 594073

On 11/6/2016 7:27 AM, Nox_firegalaxy wrote:
> After looking some time again:
> One the one hand you are completely right. Neither killing the undo/redo
> list nor the need to annotate everything is something one wants for
> "simple" highlighting. On the other hand using the exact netlist export
> process garantees that the highlighting is consistent with the exported
> netlist. So is introducing a parallel procedure with temporary
> annotation (which hopefully does not break something else) and avoidence
> of the cleanup really the best option? What if someone changes the
> behaviour only in one of both procedures?

You're finally understanding why this hasn't been implemented yet. It
turns out that is a lot more difficult than it appears.

>
> So imho if one wants to get it right one should change the netlist
> export to respect the undo/redo list - or is it good pratice that
> export/sync breaks the undo/redo?
> If I am not mistaken clearing the undo/redo list is done in (all
> eeschema files listed):
> LIB_EDIT_FRAME::OnImportPart
> LIB_EDIT_FRAME::LoadComponentFromCurrentLib
> LIB_EDIT_FRAME::CreateNewLibraryPart
> SCH_SCREEN::~SCH_SCREEN
> DIALOG_ERC::TestErc
> SCH_SCREENS::SchematicCleanUp

SchematicCleanUp() is going to be an issue. The reason it clears the
undo/redo buffer is there would be dangling wire pointers if it didn't.
This behavior could be fixed. There are several options none of which
are going to be trivial to implement. Rather than clear the undo/redo
buffer, you could create a complete wire list a push that to the undo
buffer. The problem with that is when the user attempts to undo the
changes done by SchematicCleanUp() it will appear that nothing was
undone. You could append the changes to a previous undo buffer wire
list assuming there is one but that could be tricky. The other option
would be to call SchematicCleanUp() on every wire or junction change and
add any clean up changes to the undo buffer along with the wire or
junction changes.

>
> For the here discussed case only the later three are of interested.
> Interestingly SCH_SCREEN::SchematicCleanUp is called several times
> without a correlated ClearUndoRedoList (e.g. AddNoConnect, EndSegment).
> So my question is: Is ClearUndoRedoList really required/reasonable?
> Regarding the annotation it would be better if it would not be required
> but again: to avoid double doing and later divergence in behaviour I
> would argue that it is the lesser evil to make annotation mandetory for
> net highlighting - the user is still free to not use net highlighting at
> all. Maybe I am naive but from a users perspective not having net
> highlighting at all seems to me like a bigger issue than dealing with
> annotation and even more threating is a net highlighting which is
> potentially not consistent to netlist export.

Annotation is going to be an issue as well. The cleanest way to do it
is to make a temporary copy of the entire schematic and perform a full
reannotation. This way you would be ensured a valid netlist without
interfering with the users current annotation. I don't know what kind
of overhead would be incurred or if there would be enough of a
performance penalty to make this unusable for net highlighting. You
could also write a net list generator that uses the component time stamp
which should be unique rather than the reference designator. This
should be more reliable and you would have not to annotate.

>
> P.S: Annotiation messes with undo/redo but neither clears the list nor
> does it track changes if i am not mistaken. Just apply some changes
> (e.g. rename parts), revert some changes, then run annotiation and play
> around with undo/redo.
>
> best regards
>
>
> Am 01.11.2016 um 13:06 schrieb jean-pierre charras:
>> Sorry to say that, but neither are good (and neither are acceptable).
>>
>> Unannotated components are not an issue to highlighting a connection.
>> I means the reference is not really used, although it cannot be fully ignored.
>>
>> In fact you have 2 issues, when calling prepareForNetlist() (that is currently designed to prepare a netlist for a board):
>> - it cleanup the schematic (merge collinear wires, and therefore change the wire list).
>> - it tests everything strange thing in annotation.
>>
>> Cleanup the schematic avoid side effects in connection calculation:
>> - It avoid false connections (This is a rare case, not very important here). But it has a side effect: if the wire list is modified, the undo/redo list is cleared (is has no matter when creating a netlist for a board, but it is is annoying in this case)
>> It could be avoided here.
>> - It calls CheckAnnotate().
>> CheckAnnotate() tests incorrect annotation (duplicates, not annotated and a few other things)
>> As I say previously, not annotated components are not blocking (other issues are, but they are very infrequent).
>> However annotating them before the schematic is finished is very annoying (think to not interchangeable units for instance).
>> Moreover, annotating components clears the undo/redo list.
>>
>> Therefore:
>> * prepareForNetlist(), when called to highlight a net, should not cleanup the schematic and should no test for not annotated items.
>> * Not annotated items (only to calculate the connection list) should receive a temporary annotation (for instance the full timestamp).
>> The reason is the fact if you have 2 different components having the same ref (say R?) and the same pin number (say 1) all connection to pins (R? pin 1) will be merged.
>>
>