Re: [Nano-devel] [PATCH] browser: merging opendir() in to one

[Benno Schulenberg]

On Wed, May 18, 2016, at 17:11, Rishabh Dave wrote:
> On Sun, May 15, 2016 at 5:55 PM, Benno Schulenberg
> <address@hidden> wrote:
> > Start over, and do this one step at a time.  First just move the
> > opendir() from do_browse_from() to do_browser(), [...]
> >
> > First get that right, in one patch, without changing any behavior.

You didn't do that.  You made many more changes.

> I restarted writing entire patch as per your instructions. I tested it
> and I believe there is no change in behaviour.

There is.  What if path = NULL gets passed to do_browser()?
Also, you don't free path when opendir() fails.

Further:

1) Your patch has trailing whitespace, and even a space before
a tab.  Don't do that.  (Do you even use syntax highlighting?
Those things should be glaring at you.)

2) Of several lines you only changed the indentation.  Don't
do that.

3) Your choice of variable names is poor.  In nano, everything
that starts with do_ is a function; you use it for a bool.
path_... makes me think the variable contains a path; but then
it gets set to TRUE...  Confusing.


> During this I noticed, statusbar messages disappear when we resize
> window.

That can't be helped.  If you want to read nano's messages, you
shouldn't go fiddling with the window.


So, go back, start over, and do just what I asked you to: take
just the first step.

Benno

-- 
http://www.fastmail.com - The professional email service