Re: [Nano-devel] File insertion revamp

[Benno Schulenberg]

On Thu, Feb 16, 2017, at 18:22, David Ramsey wrote:
> And now version 2c.

0001-0005: Already applied and pushed.  Slightly
modified here and there.  The comments in 0002
didn't match the code any more

0006:

Better drop the bool 'another_line'.  Just do a continue
in the else, instead of setting another_line to FALSE.  It
saves an if, and an indentation step in the rest.

+           bottomline->next = make_new_node(bottomline);
+           bottomline = bottomline->next;

Better move this to to right after:

+           bottomline->data = read_line(buf, len);

and update the preceding comment accordingly: /* Store
the data and make a new line. */

+    else {
+       bool force_another_line = FALSE;

Don't declare a variable in the middle of a function.

+           /* If it's a DOS or Mac line, strip a '\r' from it. */

The second occurrence of this comment is out of place.
Leave the if part out of it.  Just say: /* Strip the '\r'. */

I'm not sure that the whole routine doesn't create a node
too many when NONEWLINES is set.  Haven't tested it much.

0009:

+    FILE *f;
+    int descriptor;
+
+    bool old_no_newlines = ISSET(NO_NEWLINES);
+
+    filestruct *discard_top = NULL;
+    filestruct *discard_bot = NULL;

Declarations should be a single block; no blank lines.
Also, do the latter ones need to be set to NULL?

+    extract_buffer(&discard_top, &discard_bot, top, top_x, bot, bot_x);
+    free_filestruct(discard_top);

It seems that the second parameter of extract_buffer() is
seldom used.  Maybe drop it?  And only go find the last
line of the buffer when it is needed?  This is probably
best done as a follow-up patch.  The topline of the buffer
to be discarded could then simply be called 'trash'.

Benno

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