← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Fix assertion failure on attempt to orient a selected label

 

On 9/14/2015 1:03 PM, Chris Pavlina wrote:
> Well, you're welcome to try to find a non-hackish way to ensure that OnOrient is not called for nonorientable objects even when the user has selected one. I failed to do so.
> 
> Why should that be a failure, anyway? It's not illegal to attempt to orient a non-orientable object. I can still press Y and X over anything I like. The user interface just silently does nothing when I do that.
> 
> In "proper" object-oriented code, OnOrient wouldn't have the switch() in there at all, it would simply call an Orient method of the SCH_ITEM and then labels and other items that do not orient would just do nothing. What is wrong with that behavior?

Nothing.  That would be my preference.  It would require some surgery on
all items derived from SCH_ITEM and SCH_ITEM itself because OnOrient is
not defined as a virtual member of SCH_ITEM.  Only SCH_COMPONENT has an
OnOrient member.  This code is screaming "refactor me".  Do I have any
takers for a "fun" task for after the stable release? ;)

> 
> On Mon, Sep 14, 2015 at 01:00:40PM -0400, Wayne Stambaugh wrote:
>> This should probably be a wxASSERT rather than a wxFAIL_MSG so in
>> release builds it would not fail.  The assertion was put there so that
>> who ever wrote the code that allowed objects that cannot be oriented to
>> be passed to OnOrient would get a reminder of there error.  I would
>> prefer that OnOrient not get called for objects that cannot be oriented
>> rather than removing the assert.
>>
>> On 9/14/2015 10:26 AM, Chris Pavlina wrote:
>>> SCH_EDIT_FRAME::OnOrient uses SCH_COLLECTOR to filter for only orientable items. The problem is that it only does this for an unselected item. If the item is returned by SCH_SCREEN::GetCurItem it'll skip that part and go ahead trying to orient it. Then an assertion failure "Schematic object type %s cannot be oriented." is tripped.
>>>
>>> The assertion is totally unnecessary; if the object cannot be oriented it should just silently not be oriented. This patch removes the assertion.
>>>
>>> --
>>> Chris
>>>
>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~kicad-developers
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp
>>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 


Follow ups

References