nano-devel
[Top][All Lists]
Advanced

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

Re: [Nano-devel] [PATCH] Window resize handling


From: Benno Schulenberg
Subject: Re: [Nano-devel] [PATCH] Window resize handling
Date: Wed, 22 Apr 2015 20:21:50 +0200

On Wed, Apr 22, 2015, at 18:00, Mahyar Abbaspour wrote:
> Thanks for your comments on the patch. I fixed the issues you mentioned in
> the previous post including indentations. You can find the second version
> of the patch attached.

> +    /* Let the others know the handler has been executed */
> +    sigwinch_flag = sigwinch_flag ? 0 : 1;

What if there were two SIGWINCHes during a blocking period?
Wouldn't it therefore be better to just do sigwinch_flag++?

> +    if (input == KEY_WINCH) {
> +        return KEY_WINCH;
> +    }

For single-statement bodies, the braces are unneeded.  Nano's
style is not to use them in these cases.

> -     if (COLS < 32)
> -         width = COLS / 2;
> +         if (COLS < 32)
> +             width = COLS / 2;

Oh.  You made whitespace changes to unrelated lines.  Please don't
do that.  Change only lines that *have* to be changed to make your
patch work.  Whitespace I will harmonize later.

> On Wed, Apr 22, 2015 at 1:26 PM, Benno Schulenberg <address@hidden>
> wrote:
> > Well... removing this breaks the special case for Unjustify:
> > right after a ^J it is possible to do an unjustify with ^U.
> > With your patch it is no longer possible to unjustify when
> > a window resize took place right after the ^J.  With current
> > nano this works fine.  (True, a highly unlikely scenario,
> > but...).
> 
> As you have mentioned, it is very unlikely since SIGWINCH is generated by
> the user, so I thought it's unnecessary [...]

Well, I can imagine the user doing ^J, then realize that the width is
wrong, then resizes the window, and then: "Darn, I forgot to unjustify
first."  However... if it was really only the width that was wrong, the
user can do <Up> and hit ^J again to rejustify things with the new
width.

So... I will allow this tiny regression, because it's so cool to have nano
no longer jump out of the help viewer or the file browser when resizing
the window.  It's marvellous to see it adapt itself to the window width.  :)

By the way, your patch still doesn't compile.  To achieve that (on my
system at least), you will have to move #include <signal.h> from nano.c
to nano.h.

There's one little awkwardness I've found while using your patch.
Run nano, hit ^G, then make the window some twenty columns
narrower, then hit <End>.  You're not at the end of the help text.
And the other way around: hit ^G, make the window significantly
wider, then hit <End>.  You find yourself in a blank expanse.
So... ideally your patch would rebuild the help screen when a
SIGWINCH has occurred.  But if that's too diffiult/ugly/expensive
to make, I will let it pass: the user can hit ^G ^G to enter the
help screen again.

Oh, make that two awkwardnesses.  Eh, no, make this a blocker.
Start nano, hit ^R ^T, then make the window so much narrower
that you lose at least one column of files and the directory requires
two pages to display.  Then hit <PageDown>.  Huh?  It has skipped
a bunch of files.  If you use <Down> instead, the cursor wil disappear
for a little while between the two pages.  Conversely, when making
the window wider when in the file browser, and the number of
required pages is reduced, it will still display the original number
of pages, thus showing several files more than once.  So also
the file list must be regenerated when a SIGWINCH occurs
while in the file browser.

Benno

-- 
http://www.fastmail.com - Choose from over 50 domains or use your own




reply via email to

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