#173 Save search/replace history (another persistent search)

development
open
nobody
Program (79)
5
2016-07-01
2008-12-04
No

This patch adds the option to share the search/replace
history between sessions. Coming from the windows editor
Textpad, I really missed this feature all the past years.

The history file is updated on each new search/replace and
is checked on opening the find- or replace-dialog box.
It can be switched on/off through the preferences-menu
(Preferences -> Default Settings -> Searching -> Save History).

The patch is against the cvs version from 2008-12-04.

Discussion

  • Bert Wesarg

    Bert Wesarg - 2008-12-04

    Thanks for doing this.

    I see, you have taken my complaints from the other implementation into account. But you did the file format wrong. You should have taken the format from the other patch, because your \t-separated format is broken, if there is a \t inside the search/replace string.

    He used this:

    "%d:%d:%d\n%s%s\n", type, strlen(search), strlen(replace), search, replace

    I see no reason for the first \n, it should probably a ':' too. I think you get the idea, if not look at the patch.

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-04

    One more nit/pick: there are two definitions of "lastSearchdbModTime" in source/search.c. Don't know how this could compile.

     
  • Peter Muehlenpfordt

    Strange - there's only one "lastSearchdbModTime" on my side. The patch adds only one definition, too.
    How did you get the second one?

    For the file format point...
    I thought of this, but could find no way to enter a real tab in the dialog box.
    So I think, using a tab as separator is safe.
    If you enter a '\t' for regex mode it's written as this to file.
    Ok, the length/value variant is even safer. I'll try to implement it this way the next days.

    Peter

     
  • Peter Muehlenpfordt

    I found the way to enter a tab: copy&paste will do it.
    Sure, this corrupts the history file.
    The format will be changed asap. ;-)

    Peter

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-04

    Ohh, yes the double definition was on my side.

    The simpliest way to search for a tab is to select one and press Ctrl+H ;-)

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-04

    Ok, first test impression: it works ;-)

    But, there is one missing WriteSearchHistory() in case we do an incremental search. currently, only the first letter from a newly started iSearch gets into the file. The position is in this if:

    /* If the current history item came from an incremental search, and the
    new one is also incremental, just update the entry */
    if (currentItemIsIncremental && isIncremental) {
    XtFree(SearchHistory[historyIndex(1)]);
    SearchHistory[historyIndex(1)] = XtNewString(searchString);
    SearchTypeHistory[historyIndex(1)] = searchType;

    /* Save history to file */
    WriteSearchHistory();
    return;
    }

     
  • Peter Muehlenpfordt

    There's another pitfall, when searching for a newline (not \n).
    I think, I'll have to use a binary format for the history file.

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-04

    I don't think so. If you have the length, and you *have* the length, before you read the search/replace strings, you can use fread() and not fgets() which relies on the \n at the line separator.

     
  • Chris Wareham

    Chris Wareham - 2008-12-04

    Hi guys, as lebert says, I recorded the lengths in my implementation of the saved search format in order to allow any characters in the search/replace strings (tabs, newlines, etc). I was then able to fread() the search and replace strings without having to parse them for a terminator. Regards, Chris.

     
  • Peter Muehlenpfordt

    Thanks for the hints. :-)
    I have a first version with Chris' format running.
    I'll test it a bit more and post a new patch tomorrow.
    Is it really safe to mix fgets and fread? I can't find any info on this.

    Peter

     
  • Peter Muehlenpfordt

    2nd version of the patch (history format changed to support tabs and newlines in search strings)

     
  • Peter Muehlenpfordt

    Hi,

    I have just posted a new version of the patch.
    The format is the one Chris used, including the newline between length and strings.
    I think it's the easier way to read the values first.

    Mixing fgets and fread seems to be no problem, they are in sync in every case
    I've tested. I found a warning about mixing fread and the low level read, so I think
    that's the one I remembered...

    Peter

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-06

    I just want to let you know, that I get core dumps if I start nedit now. after deleting search.history it starts. I haven't debugged it yet and its still the first version. The only hint I have is that its in the strchr() function.

    I report back if I has something.

     
  • Nobody/Anonymous

    Please try the v2-patch.
    There is no strchr call any more in the new code.

    Peter

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-06

    Ok, this happens if a search line is malformed, for example, only a search type. This should not happen with version 2.

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-06

    To version 2:

    Your printf/scanf formats and arguments are buggy, strlen() returns size_t, but you request only to print a %d, which is not the same on 64bit architectures, the same fault is at scanf, you request %d but provide a pointer to size_t. Here are the warnings from gcc (the line numbers are probably differently on your side):

    search.c: In function 'WriteSearchHistory':
    search.c:4740: warning: implicit declaration of function 'GetPrefSaveSearchHistory'
    search.c:4771: warning: format '%d' expects type 'int', but argument 4 has type 'size_t'
    search.c:4771: warning: format '%d' expects type 'int', but argument 5 has type 'size_t'
    search.c: In function 'ReadSearchHistory':
    search.c:4845: warning: format '%d' expects type 'int *', but argument 4 has type 'size_t *'
    search.c:4845: warning: format '%d' expects type 'int *', but argument 5 has type 'size_t *'
    search.c:4852: warning: comparison of unsigned expression < 0 is always false
    search.c:4853: warning: comparison of unsigned expression < 0 is always false

    Why do you still have this SEARCHMAX + 1 big buffer? You know the length and you need to alloc this anyway, so alloc the buffer and fread directly into this. So the line buffer needs only to hold "%d:%d:%d\n"

    Also, I would put a seperator between search and replace string, too, so you could fread (search + 1) bytes and check if the last bytes was the seperator and overwrite this with the terminating \0.

     
  • Peter Muehlenpfordt

    How did you enable these warnings? I get the last two about the unsigned expression, but I can't get the ones about size_t (using gcc 4.2.4, tried -Wall and a few other options).
    Is '%zu' the right way to print/scan a size_t?

    I thought about reading directly to the allocated memory, but dropped the idea because I have to free the memory in case of an error. But if I re-think now - I do not free the search string, failing on reading the replace string. I should change this.

    A separating linefeed is really better to read and I don't need the line buffer anymore.

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-06

    > How did you enable these warnings? I get the last two about the unsigned
    > expression, but I can't get the ones about size_t (using gcc 4.2.4, tried
    > -Wall and a few other options).
    I use these:
    -W -Wall -Wno-unused -Wno-sign-compare -Wno-strict-aliasing

    > Is '%zu' the right way to print/scan a size_t?
    Unfortunately we can't use these, because NEdit is C89 and this is C99, so we need to cast and a %u should suffice, because SEARCHMAX is very small.

     
  • Peter Muehlenpfordt

    Strange, no chance to get this warning from my gcc...

    Posting version 3 of the patch with writing/reading uints,
    reading strings to the allocated memory and a format
    change to %d:%u:%u\n%s\n%s\n.

     
  • Bert Wesarg

    Bert Wesarg - 2008-12-15

    Thanks for the new version.

    Now to the semantics: I think if you start e incremental search (Ctrl+I), there should be a ReadSearchHistory(), too. It should be in this function search.c::BeginISearch().

     
  • Peter Muehlenpfordt

    Thanks for testing!
    Hope I didn't miss any more search/replace features.
    I never used the incremental search before, but it's really nice.

    Added the call in the function you described in patch v4.

    The history seems to work a bit different when using the inc. search, but as far as I checked the code it's a feature, not a bug. ;-)
    The position in the history isn't reset to the last item when opening the search line with Ctrl-I.

    Peter

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks