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

[Rishabh Dave]

Hello Sir,

On Wed, Nov 9, 2016 at 6:50 PM, Benno Schulenberg
<address@hidden> wrote:
>> There is one problem with this patch. After applying this patch, one
>> can open another help screen by doing ^G, ^W and ^G. Should this help
>> option be disabled or it should allow this nested help?
>
> It should be disabled; no nesting should be allowed, also because
> that help text talks about "Replace" too, and one cannot replace
> anything in a help text.
>
> Furthermore, you should temporarily unset LINENUMBERS when
> showing a help text.  And suppress the "xx lines read" message.
> And don't precede the headline with "Help: ".  And strip the
> headline (that goes into the titlebar) from the text itself,
> as it is a useless duplication.  And disable at least the ^R and
> M-J functions, and probably also the case/regex/direction toggles.

All of these are covered in the attached patch. The patch is signed.

"Switched to %s" message is also suppressed along with "Unbound key"
message in case of alphanumeric keys. 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).

> Your patch has a bug.  Run for example 'LANGUAGE=es src/nano'
> and type ^W ^G.  See how the second line of the help text ends
> with "la pantalla se Si ha".  Compare it with 'LANGUAGE=es nano'
> and see how your version has two half paragraphs missing.
> I suspect that your patch assumes that every hx[n] ends in a space.
> But this cannot be guaranteed -- translators make mistakes.

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.

> +    char *title = charalloc(50 * sizeof(char));
> +    title = fgets(title, 50, fp);
>
> Why fifty?  Use a constant, MAX_BUF_SIZE for example.

MAX_BUF_SIZE is huge for 4 or 5 words. But considering translations
I'll make it MAX_BUF_SIZE.

> +    /* Get rid of garbage value for the sake of strlen(help_text). */
> +    for (i = 0; i <= allocsize; ++i)
> +       help_text[i] = '\0';
>
> Garbage value?  How, where, what is garbage?  Probably this
> is the bug that was referred to above.

No.  Variables and allocated space contain garbage value by default.
That obstructs working of strlen(string).  However, not part of the
patch now.

> The rest I haven't tried to understand -- too much code.
> Can't you reuse the help_line_len() function?  Maybe, to
> make things easier, you should first concatenate the three
> htx-es into a single allocated string?

Can be and is done in the attached patch; but not following what you
said last to last last time - "or better when initiliazing the
help_text array: not simply strcat() the hx[] parts, but properly wrap
them first.".

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

Search Command Help Text
Go To Line Help Text
Insert File Help Text
Write File Help Text
File Browser Help Text
Browser Search Command Help Text
Spell Check Help Text
Execute Command Help Text
Main nano help text

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