[Patch] Filter Sign List (by typing a filter string) - v40

[Zuu]

2010-08-07: I've updated the explanation in the first post. For patch files please head over to the flyspray task.

This patch adds a text box at the bottom of the sign list window for filtering the sign list. Say you want to list all signs that contain "BBH", then just type BBH and only the signs that contain BBH will be displayed in the list. You can select between case sensitive and case insensitive (default) matching.

Hotkeys
f = focus the edit box of the sign list window (this hotkey is configurable)

When the edit box is selected:
Up/Down = select previous/next sign in the list
Page Up/Down = go up/down a page in the list
Ctrl + Home/End = go to the beginning/end of the list
Enter = Move viewport to selected sign or the first one in the list if none of them are selected
Escape = Clear the filter and unfocus the edit box

The reason for Ctrl + Home/End is that without Ctrl the home/end keys will move the caret of the text edit widget to the beginning/end.

You may in your hotkeys.cfg change the focus edit box hotkey to for example "GLOBAL+CTRL+L". Then you can use that hotkey to open the sign list window with the filter edit box focused.

Screenshot
This is how the, while writing this, last version of the patch looks (version 35):


The patch itself:
The patch files are found at FS#3472. Search from the end of the list of comments for the last version. Binaries are not always produced, but if they are you'll find the last one by searching from the end of this thread. Check against the last version released on flyspray to see if the binary is up to date with the last patch version.

History - edit boxes
When I started out with this patch the widget focus system was not in place in OpenTTD, instead if you had a edit box (visible or invisible!) on any window it would grab all keyboard focus. When I started with this patch I realized that it would be a no-go to add an edit box to a window that users might want to keep open for monitoring while playing along unless I would come up with a solution so that the edit box would not grab the keyboard focus when not in use. My first attempt was to make it possible for windows to toggle their query window status (a query window is a window with a text edit box) so that I could have a button on the sign list window that enables the filter function and then one to disable it again. Later I decided to try to implement a window and widget focus model where you can choose which widget to give focus. This work has been merged into trunk a year ago or so.

[XeryusTC]

This patch might be very useful, especially in #openttdcoop as there is often (read: always) referred to signs when talking about locations and this will make it A LOT easier to locate the signs.

[Zuu]

An update:

* Now the member variable search_text is cleared with '\0' on all characters, not just first character. (using memset)
* Then the correct (in oppose to before) number of characters are copied from the input buffer to search_text.
* In window.cpp: Added SignList window to list of windows that should receive all input if open, so that hotkeys don't steel input.


I think this version works well for users to use if they like. The major remaining issue is that the window steals focus always when it is open. So you can't type a hotkey when you have the sign list window open. And you must close the sign list window with your mouse as you can't use delete. (Escape = cancel search, Enter = go to sign first in list)

The attached patch applies to r14250.
(Attachment removed as newer version is available)

Edit:
Added win32 binary with the patch applied. Note that there is a glitch in the configuration menu which I think is unrelated to this patch as it appears when I compile plain r14250 in Visual Studio Express 2005. This glitch I have reported in another thread.

SignListSearch2.zip

win32 binary r14250 with sign_list_search2.patch applied.

(2.63 MiB) Downloaded 239 times

[Zuu]

Version 3:

• Includes the dynamic key foucs stealing patch.
• When you open the sign list window there is a button 'filter list' at the bottom to enable search box. Or you can use the F-key to enable search box.

  

  1.png (29.29 KiB) Viewed 13255 times

• When you cancel search (<escape> or cancel button) the search box is hided and the 'filter list' button is shown again.

  

  2.png (28.17 KiB) Viewed 13258 times

• Added global hotkey CTRL+L to open sign list.
• Changed several 'if(' to 'if (' to reflect OpenTTD coding style

Against r14250 (last nightly), includes dynamic key foucs stealing patch:
(Attachment removed as newer version is available)


My plan is that first the dynamic key foucs stealing patch have to get into trunk. Then this patch and last I want to add a feature that allows you to use arrow up/down to select a sign from the filtered result. (currently topmost sign is viewed when you hit enter)

Before this patch is ready for merging/review I need to decide on what name to use, search or filter and change the code to reflect that. The good thing with 'filter' is that the f-key is better than the s-key as s-key is heavily used to place signals. Also when you search you will get a filtered list displayed. What are your opinions on the wordings?

[WWTBAM]

I like filter. F could also stand for find.

[sulai]

Very nice idea!
Maybe this patch should be a little more general: what about ctrl+F triggers the search function in all text list windows: the station list, town list, ...?

[Zuu]

Very nice idea!
Maybe this patch should be a little more general: what about ctrl+F triggers the search function in all text list windows: the station list, town list, ...?

Nice to hear you are also excited about this feature!

That would be an extension that would be nice. But I think it's good to start out with one dialog first and then I chosen the sign list because I think that is the most important one to have search in. As signs is a more universal way to tell others in MP where to look for something.

Actually what would be neat is to make a new window that searches in all location strings (signs, towns, industries, vehicles) at the same time. Possible with toggle buttons to include/exclude groups. Put that window on Ctrl+L instead.

[Zuu]

Version 4:

* Includes dynamic steal focus patch version 3, which in addition to before makes closing of all open query windows generic.

* Renamed search -> filter in source.

* Fix: When a sign is renamed and filtering box is open => rebuild the sign list.

* Fix: Only lock arrow keys from scrolling the main viewport when filtering box is open (before it was locked always when the window was open)

* Fix: Close all query windows when showing the filter input box. (This is needed as if they would be closed by user when sign list window is in filter-mode the viewport scrolling will be unlocked and you can't move the cursor in the edit widget.)

* Fix: Clear the filter_text (previous search_text) variable when a SignList instance is created as it is used also by the sign edit window and could perhaps lead to bad things if it is not null terminated preferable with \0 on first character.

* Fix: Changed several if-statements to have the { on the same row as the if-statement as that is how OpenTTD the coding style is opposed to my coding style)

* Fix: Repaint window caption when changing filter text so that the new number of signs displayed is reflected in the caption.



Edit: Patch + Win32 binary is available at the first post in this thread, where I've made a summary of what this patch is.

[Zuu]

Version 5:

* next version (=version 4) of underlying dynamic key-focus stealing patch

* made member variable private and changed its name to m_<var-name> as stated in the coding style document

* added more comments

* I've read through the coding style now and changed the parts I though did not comply with it. Such as how to document the enum constants for widgets in a window.


(the .patch file of version 5 have been uploaded to the first post in this thread)

[planetmaker]

Hey Zuu,

I tested your patch yesterday. Great work and ease of use making the signlist much easier to handle.

Regards,
pm

[Zuu]

Thank you for testing it and looks like you enjoyed it.

Hopefully it's not impossible to get it included into the trunk. Though before the sign list patch can be worked on to get into trunk the underlying patch that make some changes to how query windows are handled need to get in trunk. But I think it is good this way that I've separated it into the first part that only make refactoring work that should not change the behavior and then this patch that implements the feature.

The changes I've made to the query window system I think most of it can be justified as making the existing code more generic and less hard coded so I think the biggest challenge is to find dev-time to do the merge.

Edit: And in case any devs reads this, you are welcome to contact me, but otherwise I will seek you when I myself have time and can't find anything to improve on the underlying key focus stealing patch. If you want to make a review then focus that first patch: http://bugs.openttd.org/task/2275

[Zuu]

Hello,

Today I've worked on an another approach that I think is more clean way to handle the problem of how to have an edit box on the sign list but not make it eat up all hotkey inputs but also so you can have multiple windows with filter boxes etc. in future.

This solution goes one step further than letting windows tell the window management system if they are a query window or not, it implements a focus model where at a given time only one window is the focused window and at each window there can only be one focused widget. So the window management checks if the focused window has a focused text input and if so, then it gives that window all key input. If the focused window don't have a focused text input it works just as usual. So this don't change how hotkeys are organized etc. Also when a window is closed, no window get the focus automatically.

http://bugs.openttd.org/task/2297


For the filter sign list patch this could mean that either the filter edit box can remain hidden and work just as now, or I can have it always available and change the f-key so it just give widget focus to the edit-box.

[Zuu]

Update patch to r14454, no other changes than that.

The patch is available at the first post in the thread.

[Zuu]

I've ported the filter sign list patch to use my widget focus patch (version 12) to allow multiple windows having text edit boxes visible at the same time. The patch and a win32 binary have been attached to this post.

Comments on the widget focus patch and/or the filter sign list patch are welcome.


On my Todo for the widget focus patch is to make the on screen keyboard (OSK) window less annoying:
* Close the OSK window when text edit looses focus
* Only open the OSK window if text edit had focus before it was clicked. (so you can click on a text edit to give it focus without having the OSK window opened)

[Zuu]

Now time for version 8:

Updated to widget focus patch 13. This update mainly contains updates to the OSK handling:

In trunk each window that contains a text edit needs to call a function to open the OSK window when the text edit is clicked. This requirement I have removed by storing the necessary parameters in QueryStringBaseWindow and in DispatchLeftClickEvent in window.cpp check if a edit box is clicked and if so open the OSK for that edit box widget.

This part I have made a separate patch of as it so far works for trunk: http://bugs.openttd.org/task/2375

In addition to above I make sure that the clicked edit box had focus previously to being clicked so that you can click ones to give an edit box focus, and to show the OSK you have to click once more. Another thing is that when focus is changing from an edit box to another widget or another window the OSK window is closed.

[planetmaker]

I just gave it a try. Nice work.

While compiling (r14509) I noticed a few warnings though:

Code: Select all

[SRC] Compiling newgrf.cpp
/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/network/network_gui.cpp: In member function 'virtual Window::EventState NetworkCompanyPasswordWindow::OnKeyPress(uint16, uint16)':
/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/network/network_gui.cpp:1809: warning: enumeration value 'HEBR_DEFAULT' not handled in switch
/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/network/network_gui.cpp:1809: warning: enumeration value 'HEBR_NOT_FOCUSED' not handled in switch

Code: Select all

[SRC] Compiling smallmap_gui.cpp
/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/signs_gui.cpp: In member function 'virtual Window::EventState SignListWindow::OnKeyPress(uint16, uint16)':
/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/signs_gui.cpp:217: warning: enumeration value 'HEBR_DEFAULT' not handled in switch[SRC] Compiling sound.cpp

/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/signs_gui.cpp:217: warning: enumeration value 'HEBR_NOT_FOCUSED' not handled in switch
/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/signs_gui.cpp: In member function 'virtual Window::EventState SignWindow::OnKeyPress(uint16, uint16)':
/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/signs_gui.cpp:424: warning: enumeration value 'HEBR_DEFAULT' not handled in switch
/Users/ingo/ottd/autostart/ps.openttdcoop.org-3979-svn/src/signs_gui.cpp:424: warning: enumeration value 'HEBR_NOT_FOCUSED' not handled in switch

Code: Select all

[SRC] Compiling yapf/yapf_road.cpp
/Users/ingo/ottd/clientpatches/src/window.cpp: In function 'void HandleKeypress(uint32)':
/Users/ingo/ottd/clientpatches/src/window.cpp:1739: warning: unused variable 'query_open'

should it matter, my compiler:
Aeolus:~/ottd/clientpatches ingo$ gcc --version
i686-apple-darwin8-gcc-4.0.1 (GCC) 4.0.1 (Apple Computer, Inc. build 5363)

[Zuu]

The first two blocks I guess is because I've exchanged constant values {0, 1, 2} with an enum since I introduce a fourth value. Adding "default: break;" to those switch-case statements could probably silence the warnings.

The last code block is an error of mine. That variable comes from the dynamic key focus stealing patch, which this patch version <= 6 used.


Thank you for posting the warnings. I should probably dig a little into MSVS and see how I can turn on more compiler warnings.

[Roujin]

The first two blocks I guess is because I've exchanged constant values {0, 1, 2} with an enum since I introduce a fourth value. Adding "default: break;" to those switch-case statements could probably silence the warnings.
[...]

If the other two values should actually never occur in those statements, "default: NOT_REACHED();" is a better way to go.

[Rainer]

Hi Zuu,

Thank you for posting the warnings. I should probably dig a little into MSVS and see how I can turn on more compiler warnings.

The user interface depends on the Version of VS.

Compiler Settings

Compiler.jpg (10.99 KiB) Viewed 11822 times

But the outcome should always be the same: Compiler Options are /WX /W4

Depending on the used libraries it may be necessary to suppress some warnings, but you can do this with a "#pragma warning disable" in the code.

cu
Rainer

[planetmaker]

I felt like fixing those warnings. Don't find attached the updated but broken patch.

EDIT: removed broken patch