On Tue, 17 May 2011 15:38:40 +0200 Colomban Wendling lists.ban@herbesfolles.org wrote:
Le 15/05/2011 14:29, Eugene Arshinov a écrit :
Hi.
When "Escape sequences" checkbox is checked in Find or Replace dialog, before escaping the find and replace strings we should save the original ones, so that they can be used in messages shown to user and in the history of those dialogs' entries. This is what the patch is about.
I described in the commit message (see the top of the patch) the changes made in the code. Just in case, here is a copy:
In the code, now we pass the original text together with the one that is actually searched for. New `original_text' field was added to GeanySearchData. A bug was fixed in document.c:show_replace_summary(): it did not escape the "No matches found for ..." string.
Hope that I did not touch plugin API with these changes. In header files I changed the following:
- document_find_text()'s signature
- document_replace_text()'s signature
- search_find_usage()'s signature
- struct GeanySearchData
Seems none of the functions you changed were in the plugin API, so it's fine. For the structure, since you appended to it, it's also fine.
However, a few comments:
- why did you used an extra string everywhere but in
document.c:show_replace_summary()? OK it already do this, but it seems not consistent since you added an "original_text".
- on search.c:on_replace_dialog_response(), prefer initialize
original_find to NULL and free it in fail rather than freeing it every time. (though anyway this function is ugly as-is with the backward goto jump just for failure)
Otherwise looks OK to me.
Attached are the original patch updated for current trunk (r5798) and another one containing changes according to your advice. In the second patch I also applied the same "technique" to show proper replace text in messages.
-- Best regards, Eugene