Re: [Nano-devel] [PATCH V8] Implement bookmarks

[Benno Schulenberg]

Op 06-12-18 om 17:17 schreef Marco Diego Aurélio Mesquita:
>  doc/nano.texi |  9 +++++++++
>  doc/nanorc.5  |  9 +++++++++

Please move the documentation changes to a separate, third patch.
They are not an essential part of making the functions work.

> +     const char *bookmark_gist = N_("Toggle bookmark");
> +     const char *nextbookmark_gist = N_("Next bookmark");
> +     const char *prevbookmark_gist = N_("Previous bookmark");

Tags (lower down) should be very short and are okay.  But the descriptions
("gists") for the help text can be (and should be) a bit more verbose.
You have some sixty characters for each of them.  So I would suggest:

"Set or remove a bookmark on the current line"
"Go to next bookmark"
"Go to previous bookmark"

(I don't want to use the word "toggle", reserving it for the feature toggles.)

Also, nano always orders the backward function first, everywhere.

> +     else if (!strcasecmp(input, "bookmark"))
> +             s->func = bookmark;
> +     else if (!strcasecmp(input, "bookmarknext"))
> +             s->func = bookmark_next;
> +     else if (!strcasecmp(input, "bookmarkprevious"))

All other bindable functions with "prev" and "next" in their names
begin with those words instead of appending them.  So the "nextbookmark"
and "prevbookmark" should do the same.

> +void bookmark_next(void);
> +void bookmark_previous(void);

to_next_bookmark()
to_previous_bookmark()

> +                                                     if (seq[3] == '3')
> +                                                             /* Esc [ 2 ; 3 
> ~ == Shift-Delete on xterm. */
> +                                                             return 
> ALT_INSERT;

The comment does not match the code.  Also, I think the returned symbol is
commentary enough.

Benno

signature.asc
Description: OpenPGP digital signature