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: Benno Schulenberg
Subject: Re: [Nano-devel] Comment/Uncomment feature patch
Date: Sat, 21 May 2016 11:21:00 +0200

On Fri, May 20, 2016, at 15:35, Mike Scalora wrote:
> Here's a first cut at a single-key comment/uncomment with the jetbrains
> heuristic (on top of my last patch). This is the logic
> ​ changes​ only
> ,
> ​I ​
> didn't change the keybinding stuff
> ​ yet​
> .

Okay.  (But please don't use HTML mail; it gets rendered weirdly
as plain text.)

> ​ Maybe ​
> is_blank_mbchar
> ​() or
> isblank
> ​() ​
> would be more correct
> ​ for testing empty lines​
> .

The first one, yes.  And you can't just s++ through a string;
you need to use move_mbright().

> ​What do you think?

Not bad.  I would use a fake undo type, say CHECK or PROBE,
so you don't have to explain what the values of action mean.
And so you don't have to make use of TRUE == 1 in the call
of comment_line().

Also, why is it necessary to disregard lines that contain only
whitespace characters?  What I can think of is: when one has
commented two blocks separately, and then wants to uncomment
them in one go (marking all the lines), and they are separated
by a line that contains only whitespace, then it would be
frustrating if M-3 commented all the lines instead of
uncommenting them.  True.

But... what is the whitespace doing there?  Programmers shouldn't
be so nonchalant.  And if they are using syntax highlighting, the
whitespace will be clearly visible, so it will not be surprising
that the lines get commented.

It would be much simpler to only disregard lines that are fully
empty, meaning: line->data[0] == '\0'.  That would stop the
magicline from being a special case, too.

(Strangely, is_blank_mbchar() doesn't consider \r to be blank, so
when editing an unconverted DOS file, a line that is factually
empty isn't considered as such.)

In do_comment(), you can reduce the messages at the end to
"The line is empty"/"The lines are empty", and the parameter
is then no longer needed, so you can rename type to action.

Benno

-- 
http://www.fastmail.com - The way an email service should be




reply via email to

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