kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Since we are asking users to run debug builds (something I'm not 100%
comfortable with but it is what it is), I went ahead and committed your
patch.  Thanks.

On 9/14/2015 1:46 PM, Chris Pavlina wrote:
> Well, personally, I don't like it - as developers looking for things to refactor aren't the only people who use debug builds. But I also see your point about not wanting it to get lost in the noise, and I'm not sure how else to manage that. I really think we ought to have a proper place to triage things like "refactor OnOrient", but that's a discussion for another time.
> 
> So, I guess, do it however you like.
> 
> On Mon, Sep 14, 2015 at 01:39:27PM -0400, Wayne Stambaugh wrote:
>> How about leaving the assert rather than the fail in there as a reminder
>> to developers in case it gets lost in the noise (which these things tend
>> to do).  It will compile away on release builds which is the desired
>> behavior.
>>
>> On 9/14/2015 1:35 PM, Chris Pavlina wrote:
>>> Oh, indeed. It's begging to be refactored. But as it stands, kicad crashes (well, throws up a dialog asking the user if they would like it to crash ;) if you press X or Y whilst holding a label. I hoped to get at least _that_ fixed before the release, because that's quite broken.
>>>
>>>
>>> On Mon, Sep 14, 2015 at 01:26:07PM -0400, Wayne Stambaugh wrote:
>>>> 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
>>>>>
>>>>
>>>> _______________________________________________
>>>> 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
> 
> _______________________________________________
> 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
>