Re: [PATCH] implementation of determine_screen_width() for Windows

[Hrvoje Niksic]

David Fritz [EMAIL PROTECTED] writes:

 It occurs to me now that the code should use STD_ERROR_HANDLE
 instead of STD_OUTPUT_HANDLE as that is where the progress bar
 goes. As it is, if stdout is redirected wget uses the default screen
 width even though output goes to stderr which is still a screen
 buffer.

OK.

 Also, there appears to have been a slight oversight when reformatting
 the patch. From looking at the code, it seems that
 determine_screen_width() will no longer return a value (zero) when
 neither TIOCGWINSZ nor WINDOWS are defined.

You're right.  This patch should fix both problems.

2004-01-29  Hrvoje Niksic  [EMAIL PROTECTED]

* utils.c (determine_screen_width): Return 0 if not running on
Windows or on a TIOCGWINSZ-capable system.

Index: src/utils.c
===
RCS file: /pack/anoncvs/wget/src/utils.c,v
retrieving revision 1.76
diff -u -r1.76 utils.c
--- src/utils.c 2004/01/28 13:42:52 1.76
+++ src/utils.c 2004/01/29 12:37:50
@@ -1674,10 +1674,12 @@
 #else  /* not TIOCGWINSZ */
 # ifdef WINDOWS
   CONSOLE_SCREEN_BUFFER_INFO csbi;
-  if (!GetConsoleScreenBufferInfo (GetStdHandle (STD_OUTPUT_HANDLE), csbi))
+  if (!GetConsoleScreenBufferInfo (GetStdHandle (STD_ERROR_HANDLE), csbi))
 return 0;
   return csbi.dwSize.X;
-# endif /* WINDOWS */
+# else /* neither WINDOWS nor TIOCGWINSZ */
+  return 0;
+#endif /* neither WINDOWS nor TIOCGWINSZ */
 #endif /* not TIOCGWINSZ */
 }
 

Re: [PATCH] implementation of determine_screen_width() for Windows

[David Fritz]

Hrvoje Niksic wrote:

This patch should fix both problems.

Great, thanks

Re: [PATCH] implementation of determine_screen_width() for Windows

[David Fritz]

Herold Heiko wrote:

From: Hrvoje Niksic [mailto:[EMAIL PROTECTED]
..

Yes.  Specifically, Unix's SIGWINCH simply sets a flag that means
window size might have changed, please check it out.  That is
because checking window size on each refresh would perform an
unnecessary ioctl.
One thing we could do for Windows is check for window size every
second or so.


I agree, but I have no idea how taxing those GetStdHandle() and
GetConsoleScreenBufferInfo() are.
Maybe David can shed more light on this, or even profile a bit.
Possibly the handle could be cached, saving at least the GetStdHandle() bit.
Heiko

Yes, GetStdHandle() would only need to be called once unless the handle 
were to change during execution (fork_to_background()?).

I haven't done any exhaustive profiling but the attached patch doesn't 
seem to affect performance. It calls determine_screen_width() every time 
the progress bar is updated (~5 times per second?).

Note: I'm not suggesting we use the patch as-is, it's just a test.

It might be possible to implement something similar to SIGWINCH using 
WinEvents, but that's not really what they were designed for. They were 
designed to be used by accessibility software (screen readers, etc.), 
and it may not be available on older versions of Windows.

How often do people change the size of the screen buffer while a command 
is running?

Index: progress.c
===
RCS file: /pack/anoncvs/wget/src/progress.c,v
retrieving revision 1.43
diff -u -r1.43 progress.c
--- progress.c  2004/01/28 01:02:26 1.43
+++ progress.c  2004/01/28 19:37:50
@@ -579,6 +579,22 @@
 /* Don't update more often than five times per second. */
 return;
 
+#ifdef WINDOWS
+{
+  int old_width = screen_width;
+  screen_width = determine_screen_width ();
+  if (!screen_width)
+   screen_width = DEFAULT_SCREEN_WIDTH;
+  else if (screen_width  MINIMUM_SCREEN_WIDTH)
+   screen_width = MINIMUM_SCREEN_WIDTH;
+  if (screen_width != old_width)
+   {
+ bp-width = screen_width - 1;
+ bp-buffer = xrealloc (bp-buffer, bp-width + 1);
+   }
+}
+#endif
+
   create_image (bp, dltime);
   display_image (bp-buffer);
   bp-last_screen_update = dltime;