Re: [Nano-devel] [PATCH] bug #48305: allow any kind of separator in "Go To Line"

[Benno Schulenberg]

On Sat, Jul 2, 2016, at 15:09, Rishabh Dave wrote:
> https://savannah.gnu.org/bugs/?48305.
> 
> Code attached in the patch makes it possible for separator to be
> anything other than '+', '-' or alphnumeric.

Hmm...  On second thought, it should probably accept a letter too
as a separator.  If I accidentally type 66m44, it should still go
to line 66, column 44.

> Also, I have tried to make it look as nice as
> possible (by breaking a single if into multiple ifs in series).

No, that is not the way to make it look nice.  When you have
multiple conditions, you string them together with && (or ||,
whatever is needed).

Also, the parsing routine should skip any leading separators
(especially whitespace).  So you first search in the input for
the first digit, and then search in the rest for the first
occurrence of something that is not a digit nor a plus nor a
minus.

Also, when changing the logic of something, do not also
rename things.  Because now I had to look hard at some lines
to make sure that it was only the name of the variable that
changed.  That is annoying.  It doesn't matter that something
is still called 'comma' but now has a much wider role.
Renaming can be done later.

Also, do you ever look at the output of 'git diff"?  Because
again you have trailing whitespace in your patch.  If you do
use 'git diff' (don't use 'less', it will do that automatically),
what is the output of 'git config color.diff'?  If it's not auto,
then set it so with 'git config color.diff auto'.

Benno

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