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

[Benno Schulenberg]

On Fri, Nov 11, 2016, at 14:44, Rishabh Dave wrote:
> All of these are covered in the attached patch. The patch is signed.

Good.

> "Switched to %s" message is also suppressed along with "Unbound key"
> message in case of alphanumeric keys.

Hm.  But if I do <AltGr+somekey>, it says: [ Unbound key: � ]
(My whole third level is occupied with UTF-8 characters.)

> And I've removed more option
> from search's bottombars, either they aren't applicable (like beg/end
> of para as lines in help text have space) or they were repetitive
> (like first line and last line).

The backward-search toggle can also go, so that ^Y and ^V get
grouped together.

> Attached patch is using help_line_len() for wrapping text while
> copying into the tempfile. So, the code responsible is no more and
> thus the bug too.

Good.

> Following are the titles of the help text and only last one has words
> uncapitalized. Should it stay that way?

Maybe, maybe not.  Not relevant for your patch.
It's a detail that can be addressed later.

Comments about details of your patch:

-               open_file(realname, new_buffer, FALSE, &f) : -2;
+               open_file(realname, new_buffer,
+                       ((currmenu == MHELP) ? TRUE : FALSE), &f) : -2;

Instead of '((currmenu == MHELP) ? TRUE : FALSE)' you can just
put 'currmenu == MHELP'.

-    if (!writable)
-       statusline(ALERT, "File '%s' is unwritable", filename);
+    if (currmenu != MHELP) {
+       if (!writable)
+           statusline(ALERT, "File '%s' is unwritable", filename);

Bwrrr...  Ugly.  You can just do:

if (currmenu == HELP)
    return;

because the rest of the routine is irrelevant for help texts.
It spares you having to reindent a bunch of stuff.  (And, I've
told you before: don't bother with whitespace, don't change
lines that don't /need/ to be changed.)

-    add_to_funcs(do_help_void, MMOST,
+    add_to_funcs(do_help_void, MMOST & ~ MHELPWHEREIS,

Why not simply exclude MHELPWHEREIS from MMOST?
And please rename the first to MFINDINHELP, becaue the
order is wrong and there are too many WHEREIS already.

-    add_to_funcs(do_first_line, 
MMAIN|MHELP|MWHEREIS|MREPLACE|MREPLACEWITH|MGOTOLINE,
-       N_("First Line"), IFSCHELP(nano_firstline_msg), TOGETHER, VIEW);
-    add_to_funcs(do_last_line, 
MMAIN|MHELP|MWHEREIS|MREPLACE|MREPLACEWITH|MGOTOLINE,
-       N_("Last Line"), IFSCHELP(nano_lastline_msg), BLANKAFTER, VIEW);
+    add_to_funcs(do_first_line, MMAIN|MHELP|MWHEREIS|MREPLACE|MREPLACEWITH|
+       MGOTOLINE|MHELPWHEREIS, N_("First Line"), IFSCHELP(nano_firstline_msg),
+       TOGETHER, VIEW);
+
+    add_to_funcs(do_last_line, MMAIN|MHELP|MWHEREIS|MREPLACE|MREPLACEWITH|
+       MGOTOLINE|MHELPWHEREIS, N_("Last Line"), IFSCHELP(nano_lastline_msg),
+       BLANKAFTER, VIEW);

Keep all the menus on a single line, and again:
don't change lines that don't *need* to be changed.

-    add_to_sclist(MWHEREIS|MREPLACE|MREPLACEWITH|MWHEREISFILE, "^P", 0, 
get_history_older_void, 0);
-    add_to_sclist(MWHEREIS|MREPLACE|MREPLACEWITH|MWHEREISFILE, "Up", KEY_UP, 
get_history_older_void, 0);
-    add_to_sclist(MWHEREIS|MREPLACE|MREPLACEWITH|MWHEREISFILE, "^N", 0, 
get_history_newer_void, 0);
-    add_to_sclist(MWHEREIS|MREPLACE|MREPLACEWITH|MWHEREISFILE, "Down", 
KEY_DOWN, get_history_newer_void, 0);
+    add_to_sclist(MWHEREIS|MREPLACE|MREPLACEWITH|MWHEREISFILE, "^P", 0,
+                       get_history_older_void, 0);
+    add_to_sclist(MWHEREIS|MREPLACE|MREPLACEWITH|MWHEREISFILE, "Up", KEY_UP,
+                       get_history_older_void, 0);
+    add_to_sclist(MWHEREIS|MREPLACE|MREPLACEWITH|MWHEREISFILE, "^N", 0,
+                       get_history_newer_void, 0);
+    add_to_sclist(MWHEREIS|MREPLACE|MREPLACEWITH|MWHEREISFILE, "Down",
+                       KEY_DOWN, get_history_newer_void, 0);

Hrrrm!  You just rewrapped stuff.  *Don't*!

+    if (ISSET(LINE_NUMBERS)) {
+       margin_copy = margin;
+       margin = 0;
+       do_toggle(LINE_NUMBERS);
+    }

Don't use do_toggle(), simply use UNSET().
And rename margin_copy to saved_margin.

Benno

-- 
http://www.fastmail.com - The way an email service should be