Re: [Nano-devel] File insertion revamp

[Benno Schulenberg]

On Wed, Feb 15, 2017, at 01:18, David Ramsey wrote:
> The only functions we have in the current code are for moving to a
> buffer and copying from a buffer.  Inserting a file is really just
> reading a file off disk into its own buffer, and then merging that
> buffer into the current file's buffer.

That is a clear description.

> The closest function for inserting a file is the one for copying from
> a buffer, but copying a buffer is unnecessary if we're merging that
> buffer.  So I needed to be able to move from a buffer in that case.  And
> the only difference between moving from a buffer and copying from a
> buffer... the former moves the original, while the latter makes a copy
> of the original and moves the copy.
> 
> Is that clearer?

It took three readings, but now I get it.  It is the "move from" that
doesn't convey that it in fact it means "insert a buffer into another".
So I would propose to call that function "ingraft_buffer()", to avoid
the word "insert" which we already use for insert_file().

And move_to_buffer() I would then call extract_buffer().

> > First, when you rename things, let's rename things properly. A
> > "filestruct" is something that does not exist.  What it refers to is
> > in fact a buffer.  So let's call those routines "move_to_buffer" and
> > "copy_from_buffer".  (Don't rename anything else -- only rename the
> > things that you change.)  A buffer is indentified by a pointer to its
> > first line.  Later on we'll rename "filestruct" to "linestruct",
> > because that is what it is.
> 
> Okay.  Although I figure splitting the two functions and renaming them
> should be separate commits then?

Yes, renaming copy_from_filestruct() everywhere would be better
as a separate commit.  But the new function gets the better name
right away.

> > Needs a comment.
> 
> Didn't put a comment there originally because that bit is taken straight
> from do_uncut_text(), which doesn't have a comment there either.  I
> guess they both need comments then?

Yes, but don't change the uncut one now -- not related.

> > Attached patch fixes that.
> 
> Thanks.  Although, since it doesn't have a git commit message, do I keep
> it as a separate patch and write one (which seems wrong since I can't
> quite imitate your writing style yet), or do I meld it with the other
> undo fixes in the existing patch (which seems wrong since I didn't write
> it)?

Meld it.  I didn't really write anything -- I just deleted superfluous
lines.  :)  Plus the change is so small... just absorb it.

> > Hm.  Some of the comments in replace_marked_buffer() make it sound as
> > if I wrote it.  :) Better not do that -- just leave out those
> > comments, as they just repeat those from replace_buffer().
> 
> So remove the comments that duplicate the ones in replace_buffer(), but
> leave the rest?

Right.

> > Was there a case where this went wrong?  Where the cursor position
> > would subtly change after a spell check?
> 
> Actually, yes.  (For the record, I'm using "aspell -c" as my alternative
> spell checker.)

I use "aspell -x -c -l en".  And I have to run nano with -Ynone, because
otherwise ^T will start the linter.

> With current git, forwards: Open files.c; move to line 8, column 10;
> turn the mark on; move forward to line 8, column 14 (so the word "nano"
> is covered by the mark), and then press ^T to start the alternative
> spell checker.  When it triggers on "nano", press "r" to replace,

My speller does not trigger on "nano" (because I've added it to the
local dictionary), so the speller immediately returns, and then...
the preceding " GNU " is selected instead.  :|  Ouch.  Bug exists
since version 2.4.1.  Only happens when the marked region is within
a single line.

> Didn't notice this until I was testing marked spell check to verify that
> it worked, or I would have brought it up earlier.  I guess a proper bug
> report is in order, then?

Yes, that would be good.

Benno

-- 
http://www.fastmail.com - A fast, anti-spam email service.