nano-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Nano-devel] [PATCH] Implement incremental search v7


From: Benno Schulenberg
Subject: Re: [Nano-devel] [PATCH] Implement incremental search v7
Date: Wed, 7 Feb 2018 19:57:08 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0


Op 07-02-18 om 12:05 schreef Marco Diego Aurélio Mesquita:
The attached patch addresses complaints from the previous one.

+/* Searches, highlights (or beeps) and positions the cursor.
+ * Possible modes are:
+ *  - SAVE_POSITION: saves current position,
+ *  - RESTORE_POSITION: restores saved position,
+ *  - SEARCH_NEXT: searches for next occurrence or
+ *  - SEARCH_FROM_START: search from the saved position.

Oof...  Ugly.

+/* Callback used by incremental search */
+void prompt_search (char *needle)
+{
+       search_text(needle, SEARCH_FROM_START);
+}

Get rid of this intermediate function.  Just call search_text()
directly.  How often do I have to say this?

-       if (keep_the_answer)
+       if (keep_the_answer && !ISSET(INCREMENTAL_SEARCH))
                sofar = mallocstrcpy(sofar, answer);

You didn't rebase to master -- patch failed to apply.

+       search_text("", SAVE_POSITION);

No.  What you need are two variables that are global to src/search.c.
Set them when a search starts, and start every search_text() from
there.

-               if (func == case_sens_void) {
+               if (func == do_research_void) {

Why do you always insist on inserting your new stuff at the top of
existing things?  Add them *at the end*.

+                       if (ISSET(INCREMENTAL_SEARCH))
+                               search_text(answer, SEARCH_FROM_START);
                } else if (func == backwards_void) {
                        TOGGLE(BACKWARDS_SEARCH);
+                       if (ISSET(INCREMENTAL_SEARCH))
+                               search_text(answer, SEARCH_FROM_START);
                } else if (func == regexp_void) {
                        TOGGLE(USE_REGEXP);
+                       if (ISSET(INCREMENTAL_SEARCH))
+                               search_text(answer, SEARCH_FROM_START);

This is silly.  When you get such code duplication, you have to insert
it somewhere else.  I've pushed a small change that probably makes this
easier.

-                                       statusbar(_("Cancelled"));
+                                       if (modus != STEPWISE)
+                                               statusbar(_("Cancelled"

No, no, no.  When canceling an Incremental search, the "Cancelled"
message should not be suppressed.

Benno



reply via email to

[Prev in Thread] Current Thread [Next in Thread]