nano-devel
[Top][All Lists]
Advanced

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

Re: [Nano-devel] Comment/Uncomment feature patch


From: Mike Scalora
Subject: Re: [Nano-devel] Comment/Uncomment feature patch
Date: Sun, 8 May 2016 11:40:30 -0600

On Tue, May 3, 2016 at 2:41 AM, Benno Schulenberg
<address@hidden> wrote:
>
> On Mon, May 2, 2016, at 14:11, Mike Scalora wrote:
>> On Sun, May 1, 2016 at 1:55 PM, Benno Schulenberg
>> <address@hidden> wrote:
>...
>
> They're still among the color commands...  :|

Sorry, I was looking at the man file. I move the comment line in all
the nanorc files now.

> You also still have this declaration inside a for statement.  Fix it.
> I can't even compile the thing when it's there.
>
> And please learn to snip all text you're not responding to.
>
>> > Comments should start with a capital and end with a period.
>> > There needs to be a space after commands like 'while', and
>> > around operands like '=='.   In the man page, sentences need
>> > to be double spaced, and better punctuated.
>
> Double-spaced sentences mean: a double space after the period.
> That's what groff does when a period is at the end of the line.
> So, for harmony, all sentences need a double space between them.

Done.

> Also, there are some functions that are not preceded by a comment.
> Each function needs one.  Further, comments among the declarations
> need to be indented and /after/ the one they refer to.  And there should
> be no * at the end of a comment line that continues on the next.

Done.

> Further, undo_comment_action() does not seem to be a good name;
> it does both undo and redo.  Maybe handle_comment_action() is better.

Changed.

> In do_comment() you don't need the variable comment_seq_len; it
> isn't used again.  Also put all declarations at the start of the function,
> not these bools in the middle.  And you don't need line_modified.
> And instead of passing a bool add_comment, wouldn't it be easier
> to just pass the action type?

Done, done & done.

>> There's one == without spaces from the iTerm2 4/2/16 patch. Do you want
>> a separate patch to fix that or let it go?
>
> Send a separate patch, if you will.

I will add it to my todo list.

> Benno

I ironed out all the enable/disable dependency issues. In the end I
decided that allowing --enable-comment with --enable-tiny was not
worth the extra complexity it would need. It would require
--enable-color, --enable-nanorc and more logic for a few other
dependencies not currently covered by existing --enable- switches.
There is now a error generated if one tries to run configure
--enable-tiny with --enable-comment regardless of the other --enable
switches.

In order to run a lot of configuration tests I wrote a bash tool that
runs 72 combinations of options (some expected error cases) and ran it
on both Ubuntu and Mac. I put the tool in a Gist in case anyone else
wants to try it in the future:
https://gist.github.com/mscalora/2fca909c88351d4920a68fd44b7dd01f if
there's a better place to put/share it, please let me know.

I used the branch w/ 'git format-patch master' method to create this
patch, let me know if that is better/worse or the same.

-Mike

Attachment: 0001-Add-new-comment-and-uncomment-feature-with-required.patch
Description: Binary data


reply via email to

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