nano-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Nano-devel] indent/unindent overhaul (including undo/redo support)


From: David Ramsey
Subject: Re: [Nano-devel] indent/unindent overhaul (including undo/redo support)
Date: Tue, 11 Jul 2017 11:40:45 -0500

On Tue, Jul 11, 2017 at 4:07 AM, Benno Schulenberg <address@hidden> wrote:
> Yes, much better, thanks.  This I can follow.  It's easy enough to
> verify that the new do_unindent() is an exact copy of do_indent(), and
> after that, each subsequent step is relatively easy to check.

Good to know.

> (I would almost be tempted to push it to master right away.)

If you want to put it in earlier, it would be one less thing I have to
maintain :)

> The two patches that change only whitespace can be a single one.

Done.

> I would have done the changes in 0003 a bit differently (you add an
> "if (!unindent)" twice when you already know what its value is; I
> would leave out the first if, and leave out the whole if-then block
> plus its comment in the second case), but fine, let it be.

Okay.

> I would shorten this to:
>
>  /* Indent the current line (or the marked lines) by tabsize columns. */

Done.

> And the rest of the comment should be wrapped properly, as you've
> changed the text:

Done.

> I would write it like this:
>
>  * Insert either a tab character or a tab's worth of spaces, depending
>  * on whether --tabstospaces is in effect. */

Done.  I also adjusted the comment in do_unindent() among similar lines.

The new version is attached.

There's one minor addition to the new version, as well: the short
descriptions in the help text for indent and unindent are updated to
make it clearer what they do.  First, indent and unindent are not exact
inverses of each other, and the help text should not imply that they
are; second, unindent's ability to remove whitespace that's a mixture of
tabs and spaces is undocumented outside the code, which is a problem.

Attachment: indent-refactor-2.zip
Description: Zip archive


reply via email to

[Prev in Thread] Current Thread [Next in Thread]