nano-devel
[Top][All Lists]
Advanced

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

Re: [Nano-devel] Patch for bug #44950


From: Benno Schulenberg
Subject: Re: [Nano-devel] Patch for bug #44950
Date: Sun, 03 Jan 2016 21:34:09 +0100

Hello Rishabh,

On Sun, Jan 3, 2016, at 16:37, Rishabh Dave wrote:
> Also, I thought over to rename patch's determine() to be more descriptive
> (determine solely sounds ambiguous as it does not tell what does it
> determine) but I could not find any name as concise as names (of functions)
> nano already has.

Don't worry.  When the time comes to integrate the patch,
I will think of something.

But... determine() returns one of two calues, either -1 or 0
-- why not make it a bool?  Also, you assign the result of
determine() to m, but never use this variable again, so...
better get rid of it.

More importantly, your patch isn't a unified diff -- it is hard
to read and impossible to apply.  Please use 'diff -ur'.

And finally, when running './nano foo/bar/baz', with your patch
nano would say that dir 'foo/bar' does not exist, whereas I would
expect it to say that dir 'foo' does not exist.  I admit, it is
harder to make, but the message would be nicer, more natural.

> All my previous patches had whitespaces so I changed my rcfile of text
> editor I am currently using.

Are you saying that... you're *not* using *nano* as your editor?!

:)

Benno

-- 
http://www.fastmail.com - Access your email from home and the web




reply via email to

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