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

[Benno Schulenberg]

On Sun, May 22, 2016, at 16:05, Rishabh Dave wrote:
> On Sun, May 22, 2016 at 1:08 AM, Benno Schulenberg
> <address@hidden> wrote:
> > Again, you changed the behavior.  But to be fair: just in theory,
> > because it is not visible.  But it will be visible if after the
> > beep() you would add statusbar("woosh"); napms(200); -- then run:
> > 'mkdir woosh && cd woosh && ../src/nano' and in another terminal:
> > 'rmdir woosh'.  Then type: ^R ^T.  Woosh.  It wouldn't have shown
> > the browser menu before your patch.
> 
> I don't understand "It wouldn't have shown the browser menu before
> your patch"-part. With my patch and your modifications it doesn't show
> browser menu.

It does.  Just for 200 milliseconds, but it does.  /If/ you had said
that you were aware that your patch would show the browser menu for
an invisibly short time when path is NULL, and that you considered
that okay (the chances being small and there is no hurry), then I
would have accepted that.  But from past submissions, you give the
impression that you don't quite know what you are doing, and that
you are too imprecise in your changes.  And that makes me bite at
every small issue in your patches.

> It just shows message "woosh".

Plus, for a moment, the browser menu.

> > Try again.  Make your patch again, with a signed-off line, and
> > a good commit message.
> 
> Umm... Do you mean that I should repeat exactly what I have done in
> last patch?

Not quite, because things have changed in git.  Show that you know
how to modify your patch to adapt to the new circumstances, and
to avoid the criticism about the momentary browser menu.

> No second step (merging rest opendir()s)

No, no second step.

Benno

-- 
http://www.fastmail.com - Does exactly what it says on the tin