nano-devel
[Top][All Lists]
Advanced

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

Re: [Nano-devel] [PATCH] enable searching (^W) in a help text (^G)


From: Benno Schulenberg
Subject: Re: [Nano-devel] [PATCH] enable searching (^W) in a help text (^G)
Date: Fri, 02 Dec 2016 12:25:39 +0100

On Thu, Dec 1, 2016, at 19:58, Rishabh Dave wrote:
> unwrapping help text to the original length when
> window's size is changed very quickly,

??

> Also I came across the solution to the issue with multibuffer

Clever.  But what if nano dies while showing a help text?
Where did the buffer contents go?  The user doesn't know.
(And no: more explanations in the manual are no good.)
So no, not acceptable.

But leave that problem for now.  And leave also this problem:
run src/nano, type ^G M-/ and resize the window -- the screen
jumps back to the start of the help text.  In current nano the
cursor will stay roughly in the same area.  It should continue
to do that.  But forget that for now.  First fix the issues that
follow below.  Then I will make a branch and it will be easier
to follow what things you are changing.

-       if (has_valid_path(realname)) {
+       if ((currmenu != MHELP && currmenu != MFINDINHELP) &&
+               has_valid_path(realname)) {

This is getting ugly.  Better introduce a global boolean,
'inhelp', set it to TRUE when do_help() is entered, and to
FALSE when exited.  This makes it much easier to understand
things like the above.  You can then also get rid of the
extra variable 'menu' in search_init().

+    /* Update the titlebar, since the filename may have changed. In case
+     * of MHELP, titlebar() in done in help.c. */

Don't mention files or functions in comments -- I hate that.
Just leave the original comment unchanged.

+bool window_went_below_76 = FALSE;
+       /* Gives the help text the original look when window size jumps
+        * from something below 76 to directly something above 76.  */

It is a mystery to me why this is needed.

+           /* Rewrap if needed, else refresh. */
+           if (COLS < 76 || window_went_below_76) {
+               window_went_below_76 = COLS < 76 ? TRUE : FALSE;
+               display_the_help_text(TRUE);
+           }

Aaah, this 76 business is an optimization...  Don't bother with
optimizations, just always recreate the help-text temp file when
the window is resized.  It makes things easier to understand.

Also, your optimization goes wrong when a translator has made
some description of a shortcut into a very long line -- the
German translation contains some overlong lines: with your
patch they are not unwrapped even when there is space.

           if (line > 0)
-               line--;
+           do_up(FALSE);
        } else if (func == do_down_void) {
-           if (line + (editwinrows - 1) < last_line)
-               line++;
+           do_down(FALSE);

Both those should use TRUE.  Pressing <Up> or <Down>
in the help text should immediately scroll the screen, not
move the cursor.

+       } else if (func == do_research) {
+           do_research();

It needs a 'focusing = TRUE' inserted to get the normal
behavior for re-searches back.

+       } else if ((kbinput >= '0' && kbinput <= '9') ||
+                       (kbinput >= 'a' && kbinput <= 'z') ||
+                       (kbinput >= 'A' && kbinput <= 'Z'))
+           /* No text can be entered at help screen. */
+           ;

This is superfluous after your change to unbound_key().

+    answer = mallocstrcpy(answer, saved_answer);
+    free(saved_answer);

Equivalent:
    free(answer);
    answer = saved_answer;

-    if (ptr < end_of_intro && COLS > 74)
+    if (COLS > 74)
        wrapping_point = 74;

Only the introductory text should be wrapped at a maximum
of 74 columns, not the descriptions of the shortcuts -- those
should be wrapped at COLS.

+    if (prompt != NULL) {
+       saved_prompt = strdup(prompt);
+       free(prompt);
+    }

This is equivalent to:
    saved_prompt = prompt;

 #ifndef DISABLE_BROWSER
-    if (path != NULL)
+    if ((currmenu == MHELP || currmenu == MFINDINHELP) && path != NULL)
+       state = _("View");

The '&& path != NULL' is superfluous.  Also, this fragment needs
to be outside of the '#ifndef'.  And instead of "View", just put
"Help" or "Help text".

Benno

-- 
http://www.fastmail.com - Or how I learned to stop worrying and
                          love email again




reply via email to

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