Re: [Nano-devel] [PATCH V4] new feature: ability to toogle and jump to bookmarks

[Brand Huntsman]

On Tue, 13 Nov 2018 10:10:27 -0200
Marco Diego Aurélio Mesquita <address@hidden> wrote:

> This adds support for rxvt and Eterm. Did we drop support for Eterm
> too?

There was a discussion and the 0x80 code was removed for ALT_DELETE in
the zap patch. You need to remove that entire chunk of code, not just part of 
the comment.


> Changes since v3:
>       - adds "placewewant = xplustabs()" to improve behaviour
>         when jumping to bookmark;
>       - removed "rxvt" from a comment.

You still haven't added support for the escape sequences I gave you.

It needs function names to rebind the keys, unless I missed them.

It would be useful if you could have multiple bookmarks per line (extremely 
long lines), but it looks like you store the bookmark in the line entry. 
Softwrap might cause confusion because an entire paragraph could be one line 
but appears to be many. And setting multiple bookmarks within it will remove 
the others. You might also miss the message that the previous bookmark was 
removed and think you added a new bookmark at current position. So if multiple 
bookmarks per line isn't supported, maybe move the bookmark within a line if 
position is different, so it requires two presses to remove a bookmark if you 
aren't sitting on the bookmark.

And you are adding 8-16 (32-64bit) bytes per line for this feature, that is way 
too much overhead. You could cut that in half by removing the boolean and using 
a negative bookmark_x to indicate no bookmark. But I would prefer you found a 
different way to implement this feature that didn't involve increasing the size 
of every line. All C files in nano are 660125 bytes and 22217 lines. This 
feature on 64bit would cause nano to use an additional 177736 bytes (27% over 
the 660125) to open those files. That is with just the bookmark_x field.