Hi Dscho
On 2016-12-23 10:30, Johannes Schindelin wrote:
Hi Beat,
On Fri, 23 Dec 2016, Beat Bolli wrote:
On 22.12.16 18:08, Johannes Schindelin wrote:
> diff --git a/compat/winansi.c b/compat/winansi.c
> index cb725fb02f..590d61cb1b 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -84,6 +84,7 @@ static void warn_if_raster_font(void)
> static int is_console(int fd)
> {
> CONSOLE_SCREEN_BUFFER_INFO sbi;
> + DWORD mode;
Nit: can we move this definition into the block below where it's used?
> HANDLE hcon;
>
> static int initialized = 0;
> @@ -98,7 +99,10 @@ static int is_console(int fd)
> return 0;
>
> /* check if its a handle to a console output screen buffer */
> - if (!GetConsoleScreenBufferInfo(hcon, &sbi))
> + if (!fd) {
Right here:
+ DWORD mode;
By that reasoning, the CONSOLE_SCREEN_BUFFER_INFO declaration that has
function-wide scope should also move below:
> + if (!GetConsoleMode(hcon, &mode))
> + return 0;
Right here.
> + } else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
> return 0;
>
> /* initialize attributes */
As the existing code followed a different convention, so does my patch.
If you choose to submit a change that moved the `mode` declaration to
narrow its scope, please also move the `sbi` declaration for
consistency.
It's probably not worth it. It just jumped at me when reading the patch,
and, writing much C++ recently, it looked weird to have a definition so
far away from the single use of the variable.
Cheers,
Beat