On Sun, Nov 26, 2017 at 3:58 PM, Jim Meyering <j...@meyering.net> wrote: > [for reference, we're discussing these: > https://debbugs.gnu.org/db/28/28083.html]
On Sun, Nov 26, 2017 at 6:58 PM, Jim Meyering <j...@meyering.net> wrote: > I made only these changes to the commit log before pushing that: > - removed capitalization and trailing period from the one-line summary > in the log message > - added ChangeLog-style "* FILENAME (function): Sentence/phrase > describing the change" line for each modified file. Great, thank you so much! > Please follow suit for your next patch(es). Will do. Here's my current NEWS entry and commit message, if you're interested. I've tried to follow the ChangeLog style. I'll send out a git format-patch when the paperwork is complete. NEWS entry: On MS-Windows, when grep is printing colorized output and is terminated prematurely by Ctrl+C, grep now restores the console's original color. [bug present since grep 2.11 added colorization for MS-Windows] Commit message: grep: reset MS-Windows console color after Ctrl+C grep implements colorized output on MS-Windows with SetConsoleTextAttribute(). However, if grep is terminated prematurely by Ctrl+C when it's in the middle of printing colorized output, the MS-Windows console won't reset the color. This results in "leaking color", where the prompt is displayed in whatever color that grep was using for highlighting. The console's color can be restored by running "color 07" (or whatever the normal color is), but the problem can be avoided completely. Fixing this problem is fairly simple. The MS-Windows API provides SetConsoleCtrlHandler(), allowing a program to intercept Ctrl+C (and Ctrl+Break, which is identically affected). On MS-Windows, Ctrl+C works by injecting an additional thread into the process. Note that this patch isn't introducing this thread - it's always injected by Ctrl+C, which uses a default handler to terminate the process. This patch simply adds an additional handler to be called. So to avoid leaking color, we need to: 1. Initialize a critical section, which cannot fail on MS-Windows Vista and newer. (On older versions of MS-Windows, it can fail under conditions of extremely low memory, which isn't a concern in practice.) 2. Whenever we're about to colorize the output, enter the critical section and check a bool (initially true) saying whether we're allowed to colorize. If so, call SetConsoleTextAttribute() while still inside that critical section. If not, do nothing. 3. During initialization, use SetConsoleCtrlHandler() to add a small function that will be called when Ctrl+C (or Ctrl+Break) is pressed. This will enter the critical section, restore the console's text color, and set the bool to false. The critical section is necessary because when MS-Windows injects the thread for Ctrl+C, the main thread can be busy toggling the color on and off. By resetting the text color and setting the bool to false, the main thread is prevented from re-colorizing the console. (The main thread also calls SetConsoleTextAttribute() at the end of colorized regions. This doesn't need to be guarded by the critical section, since repeatedly restoring the console's color is perfectly fine.) As the changes are limited to colorize-w32.c, no other platforms are affected. * lib/colorize-w32.c (color_lock): Add static CRITICAL_SECTION. (color_bool): Add static BOOL, initialized to TRUE. (w32_color_handler): When handling a Ctrl+C or Ctrl+Break event, lock color_lock, restore the console's color, and set color_bool to FALSE in order to prevent re-colorization. Return FALSE, so the default handler will be called next. (init_colorize): After initializing hstdout and norm_attr, initialize color_lock, then set w32_color_handler to handle signals for this process. (print_start_colorize): To prevent re-colorization, guard SetConsoleTextAttribute by locking color_lock and testing color_bool. * NEWS (Bug fixes): Mention this fix. Thanks, STL
