Find (Ctrl-F) loop at end of results

Bug #1845460 reported by Novak Tamas
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
KiCad
Fix Committed
Wishlist
Fabien Corona

Bug Description

When I manually solder, I open pcbnew, Ctrl-F Find, enter value e.g. "4k7", and keep pushing Enter to go along all such parts. After the last component, further Enter strikes do nothing.
For the next board I must hit Escape, Ctrl-F again, and "4k7" again. It would be nice to have a "Find first" button on the Find dialog, to rotate again through all components.
And something which is also not a bug, but a missing feature: if there are lot of same value components, the order is random. It would be better to scroll through the components in X,Y location order.

Novak Tamas (novak-7)
description: updated
Revision history for this message
Rene Poeschl (poeschlr) wrote :

[Off topic] This plugin might interest you for aiding in manual assembly: https://forum.kicad.info/t/interactive-html-bom-plugin-for-kicad-5-0/11713

Revision history for this message
Novak Tamas (novak-7) wrote :

Many thanks @Rene_Poschl, this is exactly what I need

tags: added: eeschema pcbnew ui
summary: - wish - Find Again button if pcbnew's Find
+ Find (Ctrl-F) loop at end of results
Changed in kicad:
importance: Undecided → Wishlist
milestone: none → 6.0.0-rc1
status: New → Triaged
Changed in kicad:
assignee: nobody → Fabien Corona (drinausaur)
Revision history for this message
Fabien Corona (drinausaur) wrote :

I had a look at the part of the code related to searching items. There could be a "quick" patch and have a wrap search, (that I tried and works on my machine). But I think we could do something different (and in my opinion, better): changing the search UI.

I tried to make one, I attached a screenshot.

What do you think we should go for ? simple patch for wrap-search ? Or a UI change with wrap capabilities ?
This UI would allow:
-To go back to the previous item/marker with a "find previous"
-Enable / Disable the wrap search
-Filter the markers (info, warning, error) -> Reading the doxygen I think it is possible, but didn't try yet

I think if we enable a wrap search, we should provide a text field to know when the search stars over

If you prefer the "quick" patch option, I'll upload a .patch file.

Revision history for this message
Novak Tamas (novak-7) wrote :

@Fabien
My opinion:

- No need for searching DRC markers. Whole lower part should be removed.

- If "wrap search" continues from the first after reaching last occurance, then it is not that good. I must notice that I cycled through all the components. I push a button to start again. Or wrap around is fine, if a message panel with a single default "OK" button appears, or a notification sound heard. It is important to realize when search is done.

API search functions usually have a FindFirst and FindNext function: this FindFirst should be assigned directly to a button "Search again".

"Words" is very important as searching for "R1" would find "R11", R"12" etc otherwise.

"Match case" is not usable for me, but someone may distinguish between small and capital letter names.

- Extra feature: finding objects in topological order. I don't know if it is your goal, but I miss it much.

- Extra feature: maintain a counter of search occurances (E.g. I know from BOM, that there is 30pcs of 10K, so it would be fine to count the advance)

Revision history for this message
Fabien Corona (drinausaur) wrote :

@Novak

The goal that was expressed here before is "(Ctrl-F) loop at end of results". But if we want the user to be able to enable/disable it, we have to change the GUI. Therefore, I think it is an opportunity to reconsider the function itself (should we add/change features ?)

-If the search restarts after the end of the results, I agree that the user should be notified. ( A was thinking about a message panel, or a text in the GUI, maybe with a counter, like "Hits: 1/23" ).

-In current versions, we can search for markers, do we want to remove this feature ? (that can already be found in the DRC window). I think we could remove it.

-Should we have a "find next", "find previous", and "search again" button. I am ok with these buttons.

-Agree: "Words" should stay

-"Match case" should stay

-Could you explain the topological order you suggest ? (like "reference", "value", "non-component" ?)

-I would like a counter of search occurrences, but I think it should not be using the BOM, as we can search for things that are not listed in the BOM. (we can for example search for silkscreens that are not related to components)

Revision history for this message
Novak Tamas (novak-7) wrote :

@Fabien
"If the search restarts after the end of the results, I agree that the user should be notified. ( A was thinking about a message panel, or a text in the GUI, maybe with a counter, like "Hits: 1/23" )."
I hold tweezer and soldering iron in my hands, and keep striking the Enter ("Find next" button has the focus). So after the last occurance a beep sound should be played and a modal messagebox with "Last occurance. Starting again from the first" text and with a single "OK" button should be displayed (to be dismissed with the next Enter).

"Could you explain the topological order you suggest ? (like "reference", "value", "non-component" ?)"

Please check attached video. 10k's in a row are shown in random order. The result array of a search should be sorted by anchor position (selectable XY or YX and maybe both coords ascending or descending), and displayed in order.

"I would like a counter of search occurrences, but I think it should not be using the BOM, as we can search for things that are not listed in the BOM. (we can for example search for silkscreens that are not related to components)"

Using BOM is only manual: I have BOM printed or onscreen, from where I know the amount of a particular component to solder. (e.g. 18pcs of 10k). Counter on the status line is for checking the progress ("I've done 10pcs, 8 more to go")

Revision history for this message
Novak Tamas (novak-7) wrote :

I just realized (when creating the previous attached video) that components are displayed/ordered by descending REF. Still physical coordinate would be better (IMO ...at least for manual soldering)

Changed in kicad:
status: Triaged → In Progress
Revision history for this message
Fabien Corona (drinausaur) wrote :

Patch proposal:
-Add a "wrap" option->Search results implemented as a nested list.
-Allow to go back with a "Find previous" button
-Remove the marker search option
-Fix a DLIST issue
-Add a result counter ("eg: hit(s): 1/3")
-Search history limited to 10
-Fix the search history order from previous versions
-User can include or exclude references/values/texts from results

As for the things you suggested I did not do (or did differently):
-When the "wrap" option is enabled, there is no sound / message panel, because there is the result counter.
-As for now (with this patch), there is no sorting (X-Y) algorithm, but it should be easy to implement it. (There is even a comment to locate where the algorithm should go)

Before going any further, I would like to know if this solution is accepted :)

(I tried some searches, but I don't understand what the wildcards option is supposed to do)

I also attach a picture of the search window

Revision history for this message
Fabien Corona (drinausaur) wrote :

And here is the picture

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

Hi Fabien,

1) Any reason Find Marker was removed? Is it extraneous?
2) I'd rather see the dialog get a bit wider than lose the "Match" in front of "Case".
3) I'd also be inclined to put the "Include" options on 3 separate lines so that they look visually distinct from the match options.

I gave the code a quick once-over and it looks good. I'll be out tomorrow but hopefully I'll get a chance to build and test it on Thurs.

Cheers,
Jeff.

Revision history for this message
Fabien Corona (drinausaur) wrote :

Hi Jeff,

1) Markers were not my top priority, I mainly focused on items when testing the nested list/search algorithms. I should be able to add them quite easily, with the same "Find Next"/"Find previous" mechanisms (would need some time though). I think the search criteria we can apply are quite limited (maybe levels: "Info-Warning-Errors", see picture attached with post #3). IMHO, the DRC control window is more adequate for locating DRC markers.

2) I am not a wxWidget pro, but I think this should be easy.

3) Can be done easily

I think of the patch I uploaded as a draft for discussion. (or a first patch that could be used with more patches). As soon as we decided we are aiming for, I'll patch that accordingly.

A feature that might of interest is to sort items either by reference or by location.

Revision history for this message
Novak Tamas (novak-7) wrote :

- markers: I've asked for omitting DRC markers from search, as I can't see any reason for it. I check markers visually while eliminating them; never felt for searching them, and can't see *any* purpose to do that..

- "Search again": "Restart search" for button text is more accurate.

- "Word" and "Wildcard" aren't the same (but inverted) option? When Word if ticked, searchnig for "1k" won't find "1k5". If "Word" is off, it will find "R123" when looking for "R1". How searching would behave when both "Word" and "Wildcard" is off?

- XY order: it seems results are sorted now (descending REF...or it is a natural stored order, so no sorting?) XY location order would go to the wishlist if not going to be implemented now.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote : Re: [Bug 1845460] Re: Find (Ctrl-F) loop at end of results

On 11/27/19 5:02 AM, Novak Tamas wrote:
> - markers: I've asked for omitting DRC markers from search, as I can't
> see any reason for it. I check markers visually while eliminating them;
> never felt for searching them, and can't see *any* purpose to do that..

I strongly disagree with this opinion. Why should I have to close one
dialog and open another dialog to find a different board object with
searchable text? One thing I've always like about KiCad is that things
can be done with the minimum amount extra steps. Searching for DRC
markers using the find dialog string filtering is a perfectly valid
operation. If you personally feel the need to exit the find dialog and
open the DRC dialog, that's fine but don't force that paradigm on the
rest of us. Before this patch will be accepted, the DRC marker
searching needs to be reinstated. You can implement it with a simple
checkbox "Include DRC markers" if you prefer that over a separate DRC
marker search button.

>
> - "Search again": "Restart search" for button text is more accurate.
>
> - "Word" and "Wildcard" aren't the same (but inverted) option? When Word
> if ticked, searchnig for "1k" won't find "1k5". If "Word" is off, it
> will find "R123" when looking for "R1". How searching would behave when
> both "Word" and "Wildcard" is off?

The * and ? characters are searched as is instead of used for wildcard
expansion the same as any other search. If word and wildcard are off,
"U?" would find "U?A" and "U?B" but not "U1A". You can use '*' as the
same as disabling the word option but they are not mutually exclusive.

>
> - XY order: it seems results are sorted now (descending REF...or it is a
> natural stored order, so no sorting?) XY location order would go to the
> wishlist if not going to be implemented now.
>

Revision history for this message
Fabien Corona (drinausaur) wrote :

"One thing I've always like about KiCad is that things
can be done with the minimum amount extra steps. Searching for DRC
markers using the find dialog string filtering is a perfectly valid
operation. "

-> As of today, this is not implemented. The string does not affect a search for markers. If we want to enable a search for markers using the string filtering, I can add a checkbox for markers (just like Texts, Values, References).

So for the next patch:
- "Search Again" button becomes "Restart Search"
- Implement a string filtering for DRC markers. (using a checkbox)
- Checkbox for the "Include" section on separate lines
- Dialog becomes a bit wider

Would that be ok ?

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

@Fabien, I am fine with restoring the DRC searching in a second patch provided you are willing to do so. I would like to see you incorporate Jeff's suggestions about the dialog layout before I commit the initial patch. This would bring it in line with the schematic editor dialog. I've attached a copy of the dialog as a reference.

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

> @Fabien, I am fine with restoring the DRC searching in a second patch
> provided you are willing to do so.

+1

> So for the next patch:
> - "Search Again" button becomes "Restart Search"
> - Implement a string filtering for DRC markers. (using a checkbox)
> - Checkbox for the "Include" section on separate lines
> - Dialog becomes a bit wider

+1

Revision history for this message
Fabien Corona (drinausaur) wrote :

Here is the second patch:
-Users can now search DRC markers with string filtering
-GUI changes for consistency with Eeschema

Let me know if you would like any change.

Revision history for this message
Fabien Corona (drinausaur) wrote :

And here is the picture

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

Fixed in revision 5d3e6e3d444c87f17641057fe17f3c7c54a0b8fb
https://git.launchpad.net/kicad/patch/?id=5d3e6e3d444c87f17641057fe17f3c7c54a0b8fb

Changed in kicad:
status: In Progress → Fix Committed
Revision history for this message
Jeff Young (jeyjey) wrote :

@Fabien,

I pushed your first two patches, along with a third changelist which fixes a few formatting issues and a couple of wxFormBuilder problems.

Thanks for your contribution to Kicad!

Cheers,
Jeff.

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

Now that we have the status line and hit counter, I think we should probably get rid of the "Not found" warning dialog....

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

+1

On 11/29/19 4:41 PM, Jeff Young wrote:
> Now that we have the status line and hit counter, I think we should
> probably get rid of the "Not found" warning dialog....
>

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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