Eeschema Find after Replace should not go back to beginning

Bug #1559258 reported by Silviu Laurentiu
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
KiCad
Fix Released
Medium
Jeff Young

Bug Description

Regarding the attached print screen from Eeschema, let's say that I need to replace the 3rd and 7th found word in the page, the problem occurs after the replace of 3rd word (on the example), then I press again Find button and the search starts from the begin, but in my opinion it should continue with the 4th. So, the issue consist in: after each replacement, the searching don't continue down to last position, it start from the begin of the page.

           This issue is presented on Linux Mint Cinnamon and Windows 7 x 64

Application: kicad
Version: 4.0.2-4+6225~38~ubuntu14.04.1-stable release build
wxWidgets: Version 3.0.2 (debug,wchar_t,compiler with C++ ABI 1002,GCC 4.8.4,wx containers,compatible with 2.8)
Platform: Linux 3.19.0-32-generic x86_64, 64 bit, Little endian, wxGTK
Boost version: 1.54.0
         USE_WX_GRAPHICS_CONTEXT=OFF
         USE_WX_OVERLAY=OFF
         KICAD_SCRIPTING=ON
         KICAD_SCRIPTING_MODULES=ON
         KICAD_SCRIPTING_WXPYTHON=ON
         USE_FP_LIB_TABLE=HARD_CODED_ON
         BUILD_GITHUB_PLUGIN=ON

Tags: eeschema
Revision history for this message
Silviu Laurentiu (silviulaurentiu) wrote :
summary: - Eeschema Find Replace - Start from first word after a replace
+ Eeschema Find Replace - Start from first word after each replacement
description: updated
Changed in kicad:
importance: Undecided → Wishlist
tags: added: eeschema
Novak Tamas (novak-7)
Changed in kicad:
status: New → Triaged
Jeff Young (jeyjey)
Changed in kicad:
importance: Wishlist → Medium
summary: - Eeschema Find Replace - Start from first word after each replacement
+ Eeschema Find after Replace should not go back to beginning
Revision history for this message
Jeff Young (jeyjey) wrote :

This isn't a wishlist, it's just a bug. A rather annoying one at that.

This is probably a poor application of cursor warping too, but we'll have much better ways to address that after moving to a GAL canvas.

Revision history for this message
Silviu Laurentiu (silviulaurentiu) wrote :

Thanks for your comment!
I agree! This is a bag and I reported as a bug.

Thanks for all!

Revision history for this message
Jeff Young (jeyjey) wrote :

This is a result of defensive code put in to keep the SCH_COLLECTOR list from becoming stale. Since it's a moderately long-lived list, trying to keep it fresh is probably a fool's errand.

Better to change the list to weak references so they're checked at look-up time, rather than trying to keep track of anything that *might* have deleted one of them.

Changed in kicad:
assignee: nobody → Jeff Young (jeyjey)
status: Triaged → In Progress
Revision history for this message
Jeff Young (jeyjey) wrote :
Changed in kicad:
milestone: none → 5.0.0-rc2
Revision history for this message
Wayne Stambaugh (stambaughw) wrote : Re: [Bug 1559258] Re: Eeschema Find after Replace should not go back to beginning

@Jeff, I took a quick look at this patch and it looks fine for when
objects are removed from the schematic but I don't see where it would
work when objects are added to the schematic. Have you tested your
changes for this case? Your weak reference solution is interesting. If
we were a c++17 project, you may have been able to use std::weak_ptr
instead. FYI, I just applied the patch and it fails to build on release
builds because Show() isn't defined for release builds. Here is the
build error:

C:/msys64/home/wstambaugh/src/kicad-trunk/eeschema/sch_collectors.cpp:354:10:
error: 'void DELETED_SCH_ITEM::Show(int, std::ostream&) const' marked
'override', but does not override
     void Show( int , std::ostream& ) const override {}
          ^~~~
make[2]: *** [eeschema/CMakeFiles/eeschema_kiface.dir/build.make:3486:
eeschema/CMakeFiles/eeschema_kiface.dir/sch_collectors.cpp.obj] Error 1

On 2/11/2018 8:34 PM, Jeff Young wrote:
> ** Patch added: "0007-Don-t-rely-on-live-pointers-in-search-data.patch"
> https://bugs.launchpad.net/kicad/+bug/1559258/+attachment/5053324/+files/0007-Don-t-rely-on-live-pointers-in-search-data.patch
>

Revision history for this message
Jeff Young (jeyjey) wrote :

There’s a pre-existing call to SetForceSearch() in SCH_EDIT_FRAME::OnModify(), which should catch most editing cases. I just don’t think it should be trusted to prevent crashes.

> On 12 Feb 2018, at 16:54, Wayne Stambaugh <email address hidden> wrote:
>
> @Jeff, I took a quick look at this patch and it looks fine for when
> objects are removed from the schematic but I don't see where it would
> work when objects are added to the schematic. Have you tested your
> changes for this case? Your weak reference solution is interesting. If
> we were a c++17 project, you may have been able to use std::weak_ptr
> instead. FYI, I just applied the patch and it fails to build on release
> builds because Show() isn't defined for release builds. Here is the
> build error:
>
> C:/msys64/home/wstambaugh/src/kicad-trunk/eeschema/sch_collectors.cpp:354:10:
> error: 'void DELETED_SCH_ITEM::Show(int, std::ostream&) const' marked
> 'override', but does not override
> void Show( int , std::ostream& ) const override {}
> ^~~~
> make[2]: *** [eeschema/CMakeFiles/eeschema_kiface.dir/build.make:3486:
> eeschema/CMakeFiles/eeschema_kiface.dir/sch_collectors.cpp.obj] Error 1
>
>
> On 2/11/2018 8:34 PM, Jeff Young wrote:
>> ** Patch added: "0007-Don-t-rely-on-live-pointers-in-search-data.patch"
>> https://bugs.launchpad.net/kicad/+bug/1559258/+attachment/5053324/+files/0007-Don-t-rely-on-live-pointers-in-search-data.patch
>>
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1559258
>
> Title:
> Eeschema Find after Replace should not go back to beginning
>
> Status in KiCad:
> In Progress
>
> Bug description:
> Regarding the attached print screen from Eeschema, let's say that I
> need to replace the 3rd and 7th found word in the page, the problem
> occurs after the replace of 3rd word (on the example), then I press
> again Find button and the search starts from the begin, but in my
> opinion it should continue with the 4th. So, the issue consist in:
> after each replacement, the searching don't continue down to last
> position, it start from the begin of the page.
>
> This issue is presented on Linux Mint Cinnamon and Windows
> 7 x 64
>
> Application: kicad
> Version: 4.0.2-4+6225~38~ubuntu14.04.1-stable release build
> wxWidgets: Version 3.0.2 (debug,wchar_t,compiler with C++ ABI 1002,GCC 4.8.4,wx containers,compatible with 2.8)
> Platform: Linux 3.19.0-32-generic x86_64, 64 bit, Little endian, wxGTK
> Boost version: 1.54.0
> USE_WX_GRAPHICS_CONTEXT=OFF
> USE_WX_OVERLAY=OFF
> KICAD_SCRIPTING=ON
> KICAD_SCRIPTING_MODULES=ON
> KICAD_SCRIPTING_WXPYTHON=ON
> USE_FP_LIB_TABLE=HARD_CODED_ON
> BUILD_GITHUB_PLUGIN=ON
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/kicad/+bug/1559258/+subscriptions

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

I fixed the release build issue and your patch seems to solve the issue
without adding any new ones so I merged your patch. Thanks.

On 2/12/2018 12:19 PM, Jeff Young wrote:
> There’s a pre-existing call to SetForceSearch() in
> SCH_EDIT_FRAME::OnModify(), which should catch most editing cases. I
> just don’t think it should be trusted to prevent crashes.
>
>> On 12 Feb 2018, at 16:54, Wayne Stambaugh <email address hidden> wrote:
>>
>> @Jeff, I took a quick look at this patch and it looks fine for when
>> objects are removed from the schematic but I don't see where it would
>> work when objects are added to the schematic. Have you tested your
>> changes for this case? Your weak reference solution is interesting. If
>> we were a c++17 project, you may have been able to use std::weak_ptr
>> instead. FYI, I just applied the patch and it fails to build on release
>> builds because Show() isn't defined for release builds. Here is the
>> build error:
>>
>> C:/msys64/home/wstambaugh/src/kicad-trunk/eeschema/sch_collectors.cpp:354:10:
>> error: 'void DELETED_SCH_ITEM::Show(int, std::ostream&) const' marked
>> 'override', but does not override
>> void Show( int , std::ostream& ) const override {}
>> ^~~~
>> make[2]: *** [eeschema/CMakeFiles/eeschema_kiface.dir/build.make:3486:
>> eeschema/CMakeFiles/eeschema_kiface.dir/sch_collectors.cpp.obj] Error 1
>>
>>
>> On 2/11/2018 8:34 PM, Jeff Young wrote:
>>> ** Patch added: "0007-Don-t-rely-on-live-pointers-in-search-data.patch"
>>> https://bugs.launchpad.net/kicad/+bug/1559258/+attachment/5053324/+files/0007-Don-t-rely-on-live-pointers-in-search-data.patch
>>>
>>
>> --
>> You received this bug notification because you are a bug assignee.
>> https://bugs.launchpad.net/bugs/1559258
>>
>> Title:
>> Eeschema Find after Replace should not go back to beginning
>>
>> Status in KiCad:
>> In Progress
>>
>> Bug description:
>> Regarding the attached print screen from Eeschema, let's say that I
>> need to replace the 3rd and 7th found word in the page, the problem
>> occurs after the replace of 3rd word (on the example), then I press
>> again Find button and the search starts from the begin, but in my
>> opinion it should continue with the 4th. So, the issue consist in:
>> after each replacement, the searching don't continue down to last
>> position, it start from the begin of the page.
>>
>> This issue is presented on Linux Mint Cinnamon and Windows
>> 7 x 64
>>
>> Application: kicad
>> Version: 4.0.2-4+6225~38~ubuntu14.04.1-stable release build
>> wxWidgets: Version 3.0.2 (debug,wchar_t,compiler with C++ ABI 1002,GCC 4.8.4,wx containers,compatible with 2.8)
>> Platform: Linux 3.19.0-32-generic x86_64, 64 bit, Little endian, wxGTK
>> Boost version: 1.54.0
>> USE_WX_GRAPHICS_CONTEXT=OFF
>> USE_WX_OVERLAY=OFF
>> KICAD_SCRIPTING=ON
>> KICAD_SCRIPTING_MODULES=ON
>> KICAD_SCRIPTING_WXPYTHON=ON
>> USE_FP_LIB_TABLE=HARD_CODED_ON
>> BUILD_GITHUB_PLUGIN=ON
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/kicad/+bug/1559258/+subscriptions
>

Revision history for this message
KiCad Janitor (kicad-janitor) wrote :

Fixed in revision 9d26253b2ca720107f209870894d5565c24d30fa
https://git.launchpad.net/kicad/patch/?id=9d26253b2ca720107f209870894d5565c24d30fa

Changed in kicad:
status: In Progress → Fix Committed
Changed in kicad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.