Re: [Nano-devel] [Request] allow deleting a marked region without affecting the cutbuffer

[Brand Huntsman]

On Mon, 15 Oct 2018 20:51:28 +0200
Benno Schulenberg <address@hidden> wrote:

> > + If append is TRUE, cuts are always appended to the cutbuffer.
> 
> > /* If cuts were not continuous, or when cutting a region,
> > clear the slate. */
> > -   if (!keep_cutbuffer || marked || cut_till_eof) {
> > +   if (!append && (!keep_cutbuffer || marked || cut_till_eof)) {  
> 
> I think the parameter should be called 'zapping', and the comment
> should read: "If zapping is TRUE, do not clear the cutbuffer."

I originally named it zap/zapping, but do_cut_text() doesn't actually do any 
zapping. The append parameter forces it to always append the cut to the 
cutbuffer, instead of clearing the cutbuffer. Naming it zapping implies it 
somehow enables zapping, which it does not. See below...


> > +   /* Load cutbuffer from ZAP undo. */
> > +   cutbuffer = openfile->current_undo->cutbuffer;
> > +   cutbottom = openfile->current_undo->cutbottom;  
> 
> Why is this needed?

Zapping never writes to the cutbuffer. If the zap is contiguous, the undo will 
have a cutbuffer from the last zap in it, otherwise it will be NULL. This loads 
that cutbuffer into the main cutbuffer so do_cut_text() can append to it, or 
create it if NULL.


> > +   /* Clear cutbuffer in undo. */
> > +   openfile->current_undo->cutbuffer = NULL;
> > +   openfile->current_undo->cutbottom = NULL;  
> 
> And why this?

update_undo() frees the cutbuffer in the undo and copies the main cutbuffer 
into the undo. They would both be the same memory, and this keeps them from 
being freed.


> > +   do_cut_text(FALSE, openfile->mark, FALSE, TRUE);
> > +
> > +   update_undo(ZAP);  
> 
> What needs to be updated in the undo item after the zapping?

It uses the same code path as update_undo(CUT). do_zap_text() saves cutbuffer, 
loads cutbuffer from undo and then restores cutbuffer, it uses the cut code for 
everything else.


> > +   const char *zap_gist = N_("Cut current line (or marked
> > region)");  
> 
> "Throw away the current line (or marked region)"

Thank you. I originally chopped the end off the cut_gist message, but it didn't 
make sense a week later when reviewing the newest patch. I forgot to fix it and 
then I forgot to attach the patches.


> >     add_to_funcs(do_copy_text, MMAIN,
> >             N_("Copy Text"), WITHORSANS(copy_gist),
> > BLANKAFTER, NOVIEW);
> > +   add_to_funcs(do_zap_text, MMAIN,
> > +           N_("Zap Text"), WITHORSANS(zap_gist), TOGETHER,
> > NOVIEW);  
> 
> This needs to be placed near the end of the list, next to savefile,
> to keep the help items paired.  And it should be BLANKAFTER instead
> of TOGETHER.

Oops! I forgot that add_to_funcs() orders help items. Should it be after 
find_bracket which is also !NANO_TINY, or should I open a new !NANO_TINY after 
savefile?

unrelated to patch...

I don't know how to fix this, but when zap is by copy, my help items are paired 
and without the zap patch they aren't. The various combinations of compile and 
possibly runtime options throw off alignment in help bar.

Have you changed the unicode arrows since 3.1? Every left/right arrow in help 
menu and help bar is invisible. I can see the up/down arrows, and I can paste 
the invisible left/right arrows to a 3.1 nano, but nano git won't show them.


> I don't like the ZAP_BSP_DEL name, but don't know a better one, yet.

Ya, ZAP was used for undo, and I realize that name looks like alphabet soup. 
But it makes some sense if you know what BSP and DEL are in that context, 
because it enables zap for those keys.