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

[Rishabh Dave]

On Sat, Nov 12, 2016 at 2:14 AM, Benno Schulenberg
<address@hidden> wrote:
>
> On Fri, Nov 11, 2016, at 14:44, Rishabh Dave wrote:
>> "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.)

I could see something similar only after I changed me character locale
to some other language not using basic Latin (or using something more
that basic Latin; I don't have a AltGr key, used Alt key instead). For
now I have disabled it. I /hope/ it is fixed.

>> 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.

Considering that we already have first line (^Y) and last line (^V) on
the bottombars of help screen, I think both of them are more redundant
on search's bottombars than backward-search. I've attached another
patch for this.

> 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'.

Done.

>
> -    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.)

I understand this is clearly a mistake in case of src/global.c
(mentioned below) but here too? In case there was no alternative of
"if (currmenu == HELP) return;" (which was the case since I am not
used to this way), in my opinion, not changing the indentation would
have been a mistake.

> -    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.

Okay, done. But rest of the search menus are named ______WHEREIS,
wouldn't changing to MFINDINHELP make searching for it little
difficult in future as it would be out of the line?

> -    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.

Okay, newline removed, menus on the same line and rest of the
statement on a new line, like before. For convenience of comparison, I
am copy pasting these lnes here -

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

> -    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*!

Here it clearly was a mistake. Sorry.

> +    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.

Done.


I went through the patch a couple of times more and modified some
lines in src/global.c to make the original and modified lines look
similar -- removed re-indenting and re-wrapping /hoping/ they are as
you want. Sorry for the inconvenience.

0001-feature-add-search-facility-to-help.patch
Description: Text Data

backwards-search.patch
Description: Text Data