[Top][All Lists]

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

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

From: Rishabh Dave
Subject: Re: [Nano-devel] [PATCH] bug #48305: allow any kind of separator in "Go To Line"
Date: Sun, 3 Jul 2016 20:28:38 +0530

On Sat, Jul 2, 2016 at 8:55 PM, Benno Schulenberg
<address@hidden> wrote:
>> 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).

On the contrary, I thought the one big line would look ugly. But,
yeah, I get it.

> 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.

What about alphabets at beginning? Parsing routine is okay with
whitespace at the beginning but not with non-whitespace characters.

> 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.

Okay, I did not consider that. I was trying to make patch look final.
That was the mistake. Applied patch is with comma.

> 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),

Mostly, yes. I would look specifically for them before attaching the
patch using nano's alt+p option.

> what is the output of 'git config color.diff'?  If it's not auto,
> then set it so with 'git config color.diff auto'.

It gives no output.

Attachment: allow-alphabets.patch
Description: Text Data

reply via email to

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