From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27812 invoked by alias); 24 Dec 2018 03:40:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 27802 invoked by uid 89); 24 Dec 2018 03:40:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=minor, awesome, awesome!, Cant X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 24 Dec 2018 03:40:27 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id EF61F11711E; Sun, 23 Dec 2018 22:40:25 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id cJ1p1FW4fIjs; Sun, 23 Dec 2018 22:40:25 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 72F48117117; Sun, 23 Dec 2018 22:40:25 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id C453B86756; Mon, 24 Dec 2018 07:40:18 +0400 (+04) Date: Mon, 24 Dec 2018 03:40:00 -0000 From: Joel Brobecker To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 03/16] Introduce ui_file_style Message-ID: <20181224034018.GG5246@adacore.com> References: <20181128001435.12703-1-tom@tromey.com> <20181128001435.12703-4-tom@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181128001435.12703-4-tom@tromey.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-SW-Source: 2018-12/txt/msg00291.txt.bz2 On Tue, Nov 27, 2018 at 05:14:22PM -0700, Tom Tromey wrote: > This introduces the new ui_file_style class and various helpers. This > class represents a terminal style and provides methods for parsing and > emitting the corresponding ANSI terminal escape sequences. > > gdb/ChangeLog > 2018-11-27 Tom Tromey > > * unittests/style-selftests.c: New file. > * ui-style.c: New file. > * ui-style.h: New file. > * ui-file.h: Include ui-style.h. > * Makefile.in (COMMON_SFILES): Add ui-style.c. > (HFILES_NO_SRCDIR): Add ui-style.h. > (SUBDIR_UNITTESTS_SRCS): Add style-selftests.c. FWIW, this looks good overall. I have one minor comment, and a question or two... Having the unit tests is absolutely awesome! > +/* See ui-style.h. */ > + > +void > +ui_file_style::color::get_rgb (uint8_t *rgb) const > +{ > + if (m_simple) > + { > + /* Can't call this for a basic color. */ I am trying to understand this comment, as well as the corresponding comment describing this method (in ui-style.h): + This may not be called for simple colors or for the "NONE" + color. */ Does it look like you can, in fact, call this method on a simple color? > + ui_file_style () > + { > + } Sorry for the obvious C++ question - Does this constructor means that the default constructor results in an object with undefined data? Or is that the same as... ui_file_style () = default; It seems that it would be the latter case, but then I don't see why we would want that... > +/* Skip an ANSI escape sequence in BUF. BUF must begin with an ESC > + character. Return true if an escape sequence was successfully > + skipped; false otherwise. In either case, N_READ is updated to > + reflect the number of chars read from BUF. */ > + > +extern bool skip_ansi_escape (const char *buf, int *bytes); I think there might be a copy-pasto in the comment: N_READ -> BYTES? -- Joel