[search] Unable to search in xpad pad

Bug #452226 reported by Siergiej Riaguzow
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Xpad
Status tracked in Trunk
Trunk
Fix Committed
Wishlist
Arthur Borsboom

Bug Description

There seems to be no button/menu item/keyboard shortcut to search for text in xpad pad window

Tags: search

Related branches

Changed in xpad:
status: New → In Progress
assignee: nobody → Sergei Riaguzov (riaguzov)
importance: Undecided → Wishlist
tags: added: search
Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

A possible approach of implementation has been suggested here:

https://bugs.launchpad.net/xpad/+bug/1349838

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

After the convert to GtkSourceView and GtkSourceBuffer, it becomes easier to implement the search functionality.

Create a pop-up window with search options.
Assign CTRL-F and a menu item to this window.
Then, depending on the options, the following Gtk functionality can be used.

https://developer.gnome.org/gtksourceview/stable/GtkSourceSearchContext.html

Gtk minimal version: 3.10

summary: - Unable to search in xpad pad
+ [search] Unable to search in xpad pad
Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

I don't know how to link bugs in launchpad UI but it should wait while this is done https://bugs.launchpad.net/xpad/+bug/1349838 (and it is sort of done, just needs some cleanup)

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

Hi Arthur,

Can you please review and if OK commit to master implementation of search?

https://bazaar.launchpad.net/~riaguzov/xpad/add_search/revision/1006

If you are wondering why I've removed pointers from XpadToolbar, the answer is:

If 0 is used for class_offset subclasses cannot override the class handler in their class_init method by doing super_class->signal_handler = my_signal_handler. Instead they will have to use g_signal_override_class_handler(). (c) https://developer.gnome.org/gobject/stable/gobject-Signals.html#g-signal-new

so they are useful only in case of someone wishes to inherit from XpadToolbar and I cannot find any sensible reason why someone would want to do this.

Please also note that searchbar appears in the top right corner for two reasons:

- This is how gedit does it
- This actually makes sense. searchbar is in overlay and it covers text which is searched. In the beginning of the document first several lines are usually short (for example title and an empty line after it), so putting searchbar in top right corner creates less chances for covering what is searched

Please also note that there are two key difference from gedit implementation and this one:

- In gedit searchbar dissapears if user moves cursor to text buffer. Here it is not the case. I found gedit implementation irritating in case I want to search for something, change some text, searchg for the same thing again. As xpad is not a full-blown text editor there is no search and replace functionality (and I don't know if it is needed?) but not hiding searchbar when it looses focus is at least a step to being a better text editor
- In gedit there is a complex logic to highlight search entry red if nothing is found. It is done on the fly with timers, because searching of the buffer is asynchronous and gtk_source_search_context_get_occurrences_count might not return search count immediately, so they set a timer (but not long enough to allow changing text in search entry). To avoid this complex logic I highlight search entry red only when enter/prev/next are actually clicked. If text in search entry is modified it is "unhiglighted". Also if user will move cursor to text buffer and insert the previously not found text, the search entry will stay "highlighted red" but only until enter/prev/next is pressed again on it - then it will find previously not found text

Please also note that wrap-around is set to both search forward and backward (enter and "down" button search forward, "up" button search backward). This is done to ease life. With wrap-around there is no need to grey out the prev(up)/next(down) buttons, they will just wrap-around or highlight text entry if nothing is found. Also I'm seeing wrap-around behavior in other editors like vim.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

I have reviewed the patch (rev 1006) and found the following.

- Implementation of the search
- Addition of TODO's
- Code style cleanups
- Change of the default width and height for new pads from 300 -> 200

The last point is unrelated to this search implementation.

I have applied the patch to the main branch (undoing the default width + height), including an addition to the Changelog.

On my workstation a quick test showed Xpad worked nicely.
Awesome.

Revision history for this message
Siergiej Riaguzow (riaguzov) wrote :

I'm not sure what to do with bugs in state "Fix committed" so I'm assigning it to Arthur.

For the width and height please see private e-mail. There is a duplication in xpad_settings_init (200,200) and xpad_settings_class_init (200,200) and a long TODO in xpad_settings_init. I must admit I don't understand any of that so maybe it is good that it wasn't merged in. Someone who knows better should fix it. Currently there is this 300,300 vs 200,200 but indeed it has nothing to do with search.

For TODOs please also see the e-mail. I will repeat part of it here:

Anyway the TODO is there for investigating what is the convention and to follow it. If the convention is to return GtkWidget* from constructor AND to store GtkWidget* in *Private - we should follow that. If it is to return GtkWidget* from constructor BUT store proper pointer types in *Private - we should follow that instead. Currently there is no consistency in this regard. I promise to investigate this code style issue.

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.