Find / Replace dialog

JFTR: the replace text and fonts dialogs are external python-script based extensions, 'Edit > Find…' for selecting objects is based on internal code.

I'm not convinced that those should be merged as shown in the screenshot - 'Edit > Find…' is more about creating selections than replacing the content of attributes or style properties AFAIU, and I don't see that feature supported in the unified dialog.

There have been several feature request filed, about improved selection as well as a feature to replace certain attribute/properties or text content. I'll provide links as time permits.

Feature requests with regard to refined selection methods:
Bug #170378 “Select All by Stroke or Fill Color”
Bug #171685 “Smart Select”
Bug #456180 “find dialog doesn't allow to search for attribute values”
Bug #722625 “Select objects by type, e.g. select all text objects”

Selection sets:
Bug #170286 “sets (named selections)”
Bug #314896 “Selection sets”

Feature requests with regard to 'Replace':
Bug #170259 “Add text search and replace feature”
Bug #171007 “Font Find and replace added to the Text dialog”
Bug #171962 “XML Editor - Search/Replace”
Bug #172028 “Replace Object(s)”

Other possibly related requests (mostly related to changing aspects of color on a selection of objects, or individual font style properties):
Bug #171181 “group colour/color changes, brightness, contrast, HSV, more”
Bug #171589 “dialog for adjusting colors in selection (HSL, brightness/contrast,etc)”
Bug #345586 “changing font property for multiple objects”
Bug #516108 “Tool to re-map and delete a gradient”

Requests to improve existing 'Edit > Find…':
Bug #456180 “find dialog doesn't allow to search for attribute values”
Bug #480847 “Search objects tool: checkboxes should be unchecked as default”
Bug #831012 “Selecting line width with the Find Dialog, it could bee really god”
Bug #592323 “dockable text and font settings” (comment #2)

Thanks for poiting out all the relevant re

Thanks for pointing out all the relevant request relating to Edit > Find, there certainly are a few requests ...

I've attempted to improve the find dialog and include as many of those requests as i can.

So what i did was take the current Edit > Find dialog and added the following functionality ...

1. Merged in the Replace text functionality (Bug #170259)
2. Merge in the Find and Replace font functionality (Bug #171007)
3. Added Case sensitivity and Exact match functionality
4. Added find/replace history
5. Made it a dockable dialog (Bug #592323)
6. Added search by attribute (Bug #456180)

No existing functionality has been removed from the Find dialog.

> No existing functionality has been removed from the Find dialog.

Some quick notes/questions:
- Usage:
How do I quickly select all circles to move them onto a new layer (object type)?
How do I select all objects with a certain (sub)string in their IDs to group them (ID)?
How do I identify all objects which have a clip-path or a mask, or a path effect applied (attribute name)?

- 'Search in > Attribute/Style':
Does it search in the content of all attributes of all objects within the scope? Currently this is separated into two search entries: 'Attribute' only searches in attribute names, 'Style' only searches the content of the 'style' attribute of all objects within the current scope.
How does one select objects by attribute names, partial or full match? (e.g. those with attributes in a custom namespace; or those with attributes like 'clip-path', 'mask' or 'inkscape:path-effect'; …)
Would it be intended to allow to search for matches of certain attribute values? (e.g. select all objects which refer to the same clip-path by partial id, but ignore partial matches in other attributes; or select by the value of rx or ry of ellipses; …)

- Fonts of text objects created in Inkscape are stored in two properties of the style attribute ('font-family', '-inkscape-font-specification'): what happens if I enter a font family name, and choose 'Search in > Attribute/Style' - will the search fail or does it not matter whether one uses 'Style/Attribute' or 'Font' for 'Search in'?

- If 'Font' is singled out as individual property of the style attribute, shouldn't there be separate explicit find (and replace) functions for other style properties? Like fill and stroke attributes (color for each of them; stroke width, line type (dashes), (start, mid, end) markers for stroke), opacity for object, fill and stroke, …?

@~suv
Based on your suggestions I've modified the dialog so you can select the object properties (style, attribute name, attribute value, id, fonts etc) separately.
Please check the attached screenshot, showing all the functionality.

> How do I quickly select all circles to move them onto a new layer (object type)?
The object types (including circles) are available under "More options" (see attached).
The idea was to balance having a simple dialog with advanced functionality, so the most common fields are visible and the less used fields available when needed.

> How do I select all objects with a certain (sub)string in their IDs to group them (ID)?
Find: '(sub)string' + 'Search in > Properties' + 'ID' checkbox selects those objects.

>How do I identify all objects which have a clip-path or a mask, or a path effect applied (attribute name)?
Find: 'path-effect' + 'Search in > Properties' + 'Attribute Name' checkbox selects those objects.

> 'Search in > Attribute/Style': Does it search in the content of all attributes of all objects within the scope?
Yes it searches in the attribute name or attribute value depending on the checkboxes you have on
You can refine your search results for multiple attributes - Search for an attribute, then search within 'Selection' on a different attribute.

> How does one select objects by attribute names, partial or full match? (e.g. those with attributes in a custom namespace; or those with attributes like 'clip-path', 'mask' or 'inkscape:path-effect'; …)
Find: 'path-effect' with 'Search in > Properties' and 'Attribute Name' checkbox on (default) selects those objects.
'Exact match' checkbox for partial or full name match.

 > Would it be intended to allow to search for matches of certain attribute values? (e.g. select all objects which refer to the same clip-path by partial id, but ignore partial matches in other attributes; or select by the value of rx or ry of ellipses; …)
Yes i've added the ability to search by attribute value as well - checkbox 'Attribute Value'

> what happens if I enter a font family name, and choose 'Search in >Attribute/Style' - will the search fail or does it not matter whether one uses 'Style/Attribute' or 'Font' for 'Search in'?
The font search searches inside the style but only for the 2 styles 'font-family' and '-inkscape-font-specification'.
So a search for 'Arial' will probably get the same result in 'Style' and 'Font', but a search for '96px' will only get matches in 'Style'.

> If 'Font' is singled out as individual property of the style attribute, shouldn't there be separate explicit find (and replace) functions for other style properties?
I included 'Font' because there was demand from users and a specific extension for it.
We can easily add (fill and stroke) or remove (font) properties as well.

Being able to find/replace on Styles and Attribute Value gives the users complete power to modify anything, some more specific requests it covers are:
* Bug #170378 “Select All by Stroke or Fill Color” - Yes you can do this
* Bug #171685 “Smart Select” Yes you can change the color of all the 'orange' objects to 'blue'
* Change the font size / font style for all objects at onc...

This one is starting to be a bit complex, with lots of related reports. I suggest that we use the existing blueprint (https://blueprints.launchpad.net/inkscape/+spec/find-replace-dialog) by Alexandre Prokoudine to link all the reports and use the wiki to enhance the dialog specs.

Here is a patch that covers quite a few of the ideas in the blueprint (thanks for the link jazzynico).

From the blueprint - not touched since 2008 ;)
> Inkscape should provide several presets for those who just want to do very simple replacements: replace text to text, replace color to color etc. (this will eliminate at least two existing extension effects).

This is exactly what i have tried to do ... this patch certainly doesn't provide a full 'regex' type solution as proposed, but you can find/replace on any attribute of any object so it does bring a lot of power to the users.

I think it makes a sizable step forward from the current interface.
Take a look, and decide if its worth pursing.

Agreed, that looks like a good idea ~suv. Heres a patch for that.

Or, we could even give 'System Info' its only page in the left panel.

Please ignore comment above, different issue.

Apparently, some recent changes in the trunk broke the patch.
John, would you be willing to take a look?

Updated patch attached.
Currently you can find/replace any attribute on any object, with a few common requested 'presets' included.
Please let me know any ideas to improve or simplify this dialog.

First tests (Find only) look good to me. Couple of quick questions:

1) Remnants of prior search
After a prior search, I still had a text string in the 'Find' entry, but I wanted to quickly select all paths (within nested groups), and deactivated the prior single checked property 'Attribute name':

> Find: "clip"
> Replace:
>
> Search for text in
> [x] Properties
>
> Properties:
> all deactivated
>
> Object Types:
> [x] Paths

Expected result: 633 items found
(all paths in 'icons.svg' (within nested groups) are found and selected)

Actual result: "Not Found"
(the string "foo" seems to get matched, but against which property?)

-> if all properties are deactivated, should not any find string be ignored?

2) Usability 'Object types':
After having searched for e.g. all objects of a certain type only, it happens very easily to have no 'Object Type' activated at all (and nothing gets found anymore), e.g. when the special type is deactivated, but 'All Types' not explicitly activated again.
Could there be some kind of check that 'All Types' gets activated by default if none of the other types is selected (when 'Find' is pressed), or are there other types -not listed- which are still considered for the search (not obvious to the user)?

1) > but I wanted to quickly select all paths
The Find text is now re-selected after a search, so you can quickly press the 'Delete' key to remove previous text.

> if all properties are deactivated, should not any find string be ignored ?
Yes this works, but for consistency i suggest we disable the Find/Replace buttons if either - no properties are selected, or no objects are selected. Searching for text 'something' inside properties 'nothing' should logically give nothing ?

> the string "foo" seems to get matched, but against which property?
Should not get a match against anything. Can you check in the XML Editor if "foo" is in any attribute ? I can't reproduce this.

> 2) Could there be some kind of check that 'All Types' gets activated by default
How about we disable the Find/Replace buttons if no objects are selected with status 'No objects selected' ?

Patch for above suggestions tested on Ubuntu 11.10 and attached.

Patch modified to compile after header cleanup.
Added saving of past searches to preferences file.

Tested successfully on Ubuntu 11.04, Inkscape 11002.
Another great UI improvement by John!

Testing the patch with Inkscape 0.48+devel r11002 in OS X Lion (GTK+/X11 2.24.10), I get repeated console warnings after having used the dialog once (opened, searched, closed), when opening and closing new document windows from within the same inkscape process:

  (inkscape:14092): Gtk-WARNING **: gtkwidget.c:5683: widget not within a GtkWindow

These warnings are not present when repeating the same steps with unpatched r11001.

Related gtk-2-24 source:
<http://git.gnome.org/browse/gtk+/tree/gtk/gtkwidget.c?h=gtk-2-24#n5659>

backtrace with 'G_DEBUG=fatal_warnings' attached

Thanks @suv, have fixed the warnings in this patch.
Let me know if you think it is worth committing.

See also bug #937288

Oh sorry, neglect my previous comment. Wrong place.

John, I'm still having the errors when compiling on vista 64 bit that I let you know by mail:
--------------------------
Make error line 289: problem compiling: src/ui/dialog/find.cpp: In member functi
on 'bool Inkscape::UI::Dialog::Find::item_id_match(SPItem*, const gchar*, bool,
bool, bool)':
src/ui/dialog/find.cpp:318:50: error: 'strcasestr' was not declared in this scop
e
src/ui/dialog/find.cpp: In member function 'bool Inkscape::UI::Dialog::Find::ite
m_text_match(SPItem*, const gchar*, bool, bool, bool)':
src/ui/dialog/find.cpp:358:58: error: 'strcasestr' was not declared in this scop
e
src/ui/dialog/find.cpp: In member function 'bool Inkscape::UI::Dialog::Find::ite
m_style_match(SPItem*, const gchar*, bool, bool, bool)':
src/ui/dialog/find.cpp:405:55: error: 'strcasestr' was not declared in this scop
e
src/ui/dialog/find.cpp: At global scope:
src/ui/dialog/find.cpp:422:6: warning: unused parameter 'casematch' [-Wunused-pa
rameter]
src/ui/dialog/find.cpp: In member function 'bool Inkscape::UI::Dialog::Find::ite
m_attrvalue_match(SPItem*, const gchar*, bool, bool, bool)':
src/ui/dialog/find.cpp:457:67: error: 'strcasestr' was not declared in this scop
e
src/ui/dialog/find.cpp: At global scope:
src/ui/dialog/find.cpp:479:6: warning: unused parameter 'replace' [-Wunused-para
meter]
--------------------------

I also ran cppcheck on the file and a memory leak for "text" is reported on line 546. This warning/error is not truely valid, though I would rewrite line 541 to 548 from:
--------------------------
    const gchar* text = g_strdup(entry_find.get_text().c_str());

    GSList *in = l;
    GSList *out = NULL;

    if (strlen (text) == 0) {
        return in;
    }
--------------------------

into the following:
--------------------------
   Glib::ustring tmpstring = entry_find.get_text();
    if (tmpstring.empty())
    {
        return l;
    }
    const gchar* text = g_strdup(tmpstring.c_str());

    GSList *in = l;
    GSList *out = NULL;
--------------------------

@Kris, sorry about the build error, it seems strcasestr function is not available on Windows.
I have fixed that in this patch, it now builds and runs on Windows 7.
I have also modified the code to remove the potential leak (comment #24) you mentioned.

Builds on Windows XP too.
It's a great improvement, indeed!
One note: perhaps it would be better to unselect all when after pressing the "Find" button nothing matching is found. Now the current selection is preserved and this could lead to thinking that it's the result of the search.

@LucaDC - when searching within the current selection, this might not be helpful (else you have to repeat all steps to arrive at the current selection again)

Ok, now I see there is a Scope selection. This is a nice feature I didn't see.
Anyway, IMHO the current behavior is not intuitive: maybe, to keep the functionality, this could happen when searching in the Selection scope, but remove the selection if the search is done in All scope.

And remove the selection in the Current Layer scope too (because you never lose the layer).

This new functionality indeed may have a great impact.

Comments:
- what was not in comment 24: probably a call to "g_free(text)" is needed on line 642.
- regarding my former work on Bug #170765, my attention was drawn on the new options: is it possible to add mnemonics for the "Properties" and "Object types"?
- There is a space in front of the "...". As far as I can see the default is not to have a space. However, what is the guideline here do we want this symbol? At least in the object properties dialog I c++-ified, it is missing.
- see attachment: with the current selection for colours the two top left rectangles should be selected (is correctly done): however, regarding callbacks:
+ entering a replacement in the edit box does not enable the "Replace" button on my system
+ clicking on the minus at the left of "options" does not resize the dialog to its smaller size (at least when the dialog is floating)
+ I would propose to use the canvas selection callback for updating the "xx objects found" label: if deselected no comment, etc. Is more informing as the user might delete objects that comply with one of the find options. Hence, the info in the label is not valid anymore.

Thanks for all the suggestions, an updated patch is attached.

@LucaDC - good idea, if nothing is found for All or Current Layer, then selected objects are unselected

@Kris
> a call to "g_free(text)" is needed on line 642
Thanks added

>a space in front of the "..."
Have removed the space and "..."

> does not enable the "Replace" button on my system
Replace button is disabled when you check on the "Attribute Name" property. I thought it was a bit too dangerous replacing attribute names. Do you think there a valid use case for replacing attribute names ?

> clicking on the minus at the left of "options" does not resize the dialog to its smaller size
Yes this a common problem, that occurs in several dialogs (Object Properties and Document Properties->Page), unfortunately i dont know how to fix this. Any place where this works correctly ?

> use the canvas selection callback for updating the "xx objects found" label
Sorry not quite understanding this. Do you suggest we update the "xx objects found" label when the selection on canvas changes ? Even when the user changes the selection ?

>> use the canvas selection callback for updating the "xx objects found" label
>Sorry not quite understanding this. Do you suggest we update the "xx objects found" label when the selection on canvas changes ? Even when the user changes the selection ?

What I intended is that when the amount of selected objects changes (because of an action of the user), the label should be blanked. Reasons:
- one or more objects that meet the selection criteria can have been deleted in the mean time. The info shown in the user interface should in my humble opinion allways be correct (which is not when an object is deleted and the label not updated)
- for complex documents, one might get confused whether the current selection is still the selection of objects that meet the selection criteria
- actually, when the user is informed on the number of objects that agree the selection criteria just after he presses the search button, it is all right: the objects are selected at this moment and this number of objects is also shown in the statusbar (along with the object type).

Therefore, I would use the desktop tracker to empty the label when the selection changes (e.g. no selection, other objects selected, etc.). Look for example in the code of the object attributes dialog for connectChanged (property of the desktop tracker) and selectChangedConn.

> Do you think there a valid use case for replacing attribute names ?

No. I have to admit that I'm not an svg specialist nor a graphics specialist, so I have only a limited view on niche applications.

By the way: thanks for the documentation of the dialog so far. I do not know a dedicated page so far on Inkscape documentation through doxygen, so therefore I wanted to add a little explanation. The /** */ comments are added in front of a single function to document it. To add a note in the code about several members functions or private variables, /* */ will do. /** */ can not be used for multiple consecutive functions or data members in the code, so might generate wrong HTML output. (I downloaded the GUI and the package on windows from http://www.stack.nl/~dimitri/doxygen/download.html#latestsrc)

@Kris
>I would use the desktop tracker to empty the label when the selection changes
Thanks for the suggestion. Understood i have added that in this version.

Nice result. Would like to see it committed.
Though it is a whishlist item, I noticed that not all occurrences of a string in a text box are replace. For example the string:
   dfgdfg dfg
with replacement string:
   blabla
Results in:
   blabladfg dfg
Where I would expect to see:
   blablablabla blabla
Or am I missing something in the options?

One UI suggestion that depends on the radio button "search in". If "Text" is selected, it might be better not to show the options for "Object types" (in favor) and "Properties" (do not know yet because of the font option).

> not all occurrences of a string in a text box are replace
Yes this is true. Often applications have Find / Replace / Replace All options.
How should we handle this, add a checkbox for "Replace All" or another button "Replace All" , or just Replace All by default ?

Patch that also "replaces all" instances of text, as the default.

If there is no further concerns, i will commit this to the trunk.

Minor detail/question:
Would it be possible to make the layout of the 'Properties' frame as compact with regard to vertical spacing as 'General' and 'Object Types'?

(I didn't find anything obvious in the widget tree which sets the vertical spacing between the two GtkHBoxes - obviously the other two option frames use two GtkVboxes in a GtkHBox, and the GtkCheckButtons inside those GtkVBox containers are more tightly arranged with regard to vertical spacing than two GtkHBoxes in a GtkVBox)

@suv, yes i will get the 'Properties' frame to have the same padding as the other frames, and commit the code shortly.

Committed as r11117.
Will need testing as it uses some gtkmm 2.24+ functions in Gtk::ComboBoxText.

Additional commit r11119 to fix build error.
https://launchpad.net/~inkscape.dev/+archive/trunk/+build/3312454

I made some minor adjustments to the user messages, e.g. differentiation whether one or more objects are found.

However, after working a bit with the dialog, I'm not happy about the usage of the term "object" and I wonder whether it should be "match". The reason is that it is not the object that is replaced (the same object is still there after the modification), but its property (in general the "match" is replaced) is replaced.
What do other people think? Is there even a better native word for match or object?

The term "object" does make sense when Finding, and this term is also used on the bottom statusbar when selecting.
However it is misleading when replacing text or properties. Match seems like a reasonable term to me.

Made the update in user messages in revision 11363.
I noticed however another tiny issue for user experience: it is still possible that de "replace all" button is active even when no object matches the selection criteria. Actually now, virtually always the replace button is set to sensitive when the find button is. Probably sensitive should set to false at the bottom of onToggleCheck for the replace button, whereas the status should be set to true only in the specific valid cases in onAction. Not sure about the implementation now, need to study this further ...

Fixed previous mentioned item and undo message in trunk revision 11456.