[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Nano-devel] [PATCH] browser: merging opendir() in to one
From: |
Rishabh Dave |
Subject: |
Re: [Nano-devel] [PATCH] browser: merging opendir() in to one |
Date: |
Tue, 24 May 2016 18:40:32 +0530 |
On Mon, May 23, 2016 at 1:05 AM, Benno Schulenberg
<address@hidden> wrote:
>
> 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.
>
Actually, yes. I missed browser menu as I thought menu implied "menu
that displays files & directories". I see the command-menu now that is
display for a moment at the bottom. And I realized why was that.
>> > 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.
>
Okay, I removed the cause for momentary display of browser menu (but
it is ineffective as we already return NULL when path is NULL; I did
it for the order that should be ideally) and removed check for 'path
!= NULL' as we do check for it earlier.
I added comment in last minute and that is what 2nd patch is about. I
feel some reader in future would appreciate it if he wouldn't have to
look out for why path isn't checked before use, like I did for
opendir() with respect to closedir() last time. (Should I avoid
sending two signed to patches? I did that to keep second optional.)
0001-browser-Move-opendir-closedir-to-do_browser.patch
Description: Text Data
0002-browser-Move-opendir-closedir-to-do_browser.patch
Description: Text Data
- [Nano-devel] [PATCH] browser: merging opendir() in to one, Rishabh Dave, 2016/05/14
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one, Benno Schulenberg, 2016/05/15
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one, Rishabh Dave, 2016/05/18
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one, Benno Schulenberg, 2016/05/19
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one, Rishabh Dave, 2016/05/21
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one, Benno Schulenberg, 2016/05/21
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one, Rishabh Dave, 2016/05/22
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one, Benno Schulenberg, 2016/05/22
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one,
Rishabh Dave <=
- Re: [Nano-devel] [PATCH] browser: merging opendir() in to one, Benno Schulenberg, 2016/05/25