From patchwork Sat Jan 31 08:57:19 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eli Zaretskii X-Patchwork-Id: 4856 Received: (qmail 27431 invoked by alias); 31 Jan 2015 08:57:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27418 invoked by uid 89); 31 Jan 2015 08:57:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout28.012.net.il Received: from mtaout28.012.net.il (HELO mtaout28.012.net.il) (80.179.55.184) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 31 Jan 2015 08:57:22 +0000 Received: from conversion-daemon.mtaout28.012.net.il by mtaout28.012.net.il (HyperSendmail v2007.08) id <0NJ100A009HDF800@mtaout28.012.net.il> for gdb-patches@sourceware.org; Sat, 31 Jan 2015 10:55:29 +0200 (IST) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by mtaout28.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NJ100AI7A4HLI20@mtaout28.012.net.il>; Sat, 31 Jan 2015 10:55:29 +0200 (IST) Date: Sat, 31 Jan 2015 10:57:19 +0200 From: Eli Zaretskii Subject: Re: [PATCH] TUI: Expand TABs into spaces In-reply-to: To: Doug Evans Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83wq43jpjk.fsf@gnu.org> References: <83k3149k5b.fsf@gnu.org> <837fwn2cvc.fsf@gnu.org> <83a918s6k5.fsf@gnu.org> <834mreq9t3.fsf@gnu.org> X-IsSubscribed: yes > From: Doug Evans > Cc: gdb-patches@sourceware.org > Date: Tue, 27 Jan 2015 00:06:12 -0800 > > > I did that below, but since tui_register_format is its only user, > > keeping that function in tui-regs.c would have allowed us to make it > > static. Wouldn't that be slightly better? > > That's not an invalid viewpoint, and if you want to go > that route, fine by me. One argument for going that route > is that if another user comes along one can always make > it public later. Alas that doesn't always happen, and one > ends up writing something that already exists. > [If a year from now I need something like that the last > place I'm going to look is tui-regs.c, whereas I will at least look > through all the headers and generic-looking files for something. > I wish I had a previous example at hand, I've paged out the details > from memory and all that's left is wanting to avoid that. > IIRC we've even had cases where something generic was used by > multiple callers, then only one, and then someone moved it and > made it static. If there were multiple callers once there may > be again (depending on the situation of course) so if I'm given the > choice I wouldn't have made that move.] > [Also, one could argue that this function isn't even tui-specific > and thus should go in something like gdb/utils.c or even libiberty. > Keeping it in tui just violates what I said above about being in > the last place someone would look. > I opted for not getting too carried away with things: I don't > yet see a need for expanding tabs to spaces outside of tui, > but I could be wrong of course. Plus I wouldn't impose spending > any time going back and forth picking the absolute best location, > there are far more important things, and one could argue I've made > you read too much already.] > [OTOOH :-), if I hadn't said at least some of this the probability > is too high that someone else would. Bleah. > [Not saying it would be you though!]] > > So to make a long story short, feel free to leave it where it is or > make it static. I left it where it is, given that the pros and cons are inconclusive. > > +/* Utility function to expand TABs in a STRING into spaces. STRING > > + will be displayed starting at column COL. The returned expanded > > + string is malloc'ed. */ > > blank line between function comment and definition > [I realize tui_get_register doesn't do that, but best do that with > new code.] Done. > Also, I'm guessing that newlines in STRING isn't supported here, right? > If so, mention that in the function comment. Done. > One could even add an assert like: > > gdb_assert (strchr (string, '\n') == NULL); > > or some such. I didn't do this, I think that'd be too much. Having a slightly messed-up screen is far better than having to abort the debug session, if we ever get to this problem. The mess-up will most probably be reported, and we will fix it. > > +/* Expand TABs into spaces. */ > > For reference sake, there are multiple schools of thought, and no > fixed convention in gdb, for whether to put a comment here, and > what it should look like. No need to change anything, and I can > go into details if you want. Just wanted to mention the issue, > and the acceptable choice of not having any comment here. I had in the past requests to add such comments, so at least some of the maintainers favor them. I left that comment intact. The patch as I pushed it appears below. Thanks again for reviewing it. commit 312809f8838911dabff84d7ad3ccf341307d2b19 Author: Eli Zaretskii Date: Sat Jan 31 10:47:14 2015 +0200 Make sure TABs are expanded in TUI windows on MS-Windows. gdb/ 2015-01-31 Eli Zaretskii * tui/tui-io.c (tui_expand_tabs): New function. (tui_puts, tui_redisplay_readline): Expand TABs into the appropriate number of spaces. * tui/tui-regs.c: Include tui-io.h. (tui_register_format): Call tui_expand_tabs to expand TABs into the appropriate number of spaces. * tui/tui-io.h: Add prototype for tui_expand_tabs. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 58f5a7b..51f1714 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2015-01-31 Eli Zaretskii + + * tui/tui-io.c (tui_expand_tabs): New function. + (tui_puts, tui_redisplay_readline): Expand TABs into the + appropriate number of spaces. + * tui/tui-regs.c: Include tui-io.h. + (tui_register_format): Call tui_expand_tabs to expand TABs into + the appropriate number of spaces. + * tui/tui-io.h: Add prototype for tui_expand_tabs. + 2015-01-30 Doug Evans * NEWS: "info source" command now display producer string if present. diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c index 7e8a3bc..19e9485 100644 --- a/gdb/tui/tui-io.c +++ b/gdb/tui/tui-io.c @@ -178,7 +178,20 @@ else if (tui_skip_line != 1) { tui_skip_line = -1; - waddch (w, c); + /* Expand TABs, since ncurses on MS-Windows doesn't. */ + if (c == '\t') + { + int line, col; + + getyx (w, line, col); + do + { + waddch (w, ' '); + col++; + } while ((col % 8) != 0); + } + else + waddch (w, c); } else if (c == '\n') tui_skip_line = -1; @@ -256,6 +269,16 @@ waddch (w, '^'); waddch (w, CTRL_CHAR (c) ? UNCTRL (c) : '?'); } + else if (c == '\t') + { + /* Expand TABs, since ncurses on MS-Windows doesn't. */ + getyx (w, line, col); + do + { + waddch (w, ' '); + col++; + } while ((col % 8) != 0); + } else { waddch (w, c); @@ -724,6 +747,59 @@ return ch; } +/* Utility function to expand TABs in a STRING into spaces. STRING + will be displayed starting at column COL, and is assumed to include + no newlines. The returned expanded string is malloc'ed. */ + +char * +tui_expand_tabs (const char *string, int col) +{ + int n_adjust; + const char *s; + char *ret, *q; + + /* 1. How many additional characters do we need? */ + for (n_adjust = 0, s = string; s; ) + { + s = strpbrk (s, "\t"); + if (s) + { + col += (s - string) + n_adjust; + /* Adjustment for the next tab stop, minus one for the TAB + we replace with spaces. */ + n_adjust += 8 - (col % 8) - 1; + s++; + } + } + + /* Allocate the copy. */ + ret = q = xmalloc (strlen (string) + n_adjust + 1); + + /* 2. Copy the original string while replacing TABs with spaces. */ + for (s = string; s; ) + { + char *s1 = strpbrk (s, "\t"); + if (s1) + { + if (s1 > s) + { + strncpy (q, s, s1 - s); + q += s1 - s; + col += s1 - s; + } + do { + *q++ = ' '; + col++; + } while ((col % 8) != 0); + s1++; + } + else + strcpy (q, s); + s = s1; + } + + return ret; +} /* Cleanup when a resize has occured. Returns the character that must be processed. */ diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h index 8f96cfe..3154eee 100644 --- a/gdb/tui/tui-io.h +++ b/gdb/tui/tui-io.h @@ -41,6 +41,9 @@ changed the edited text. */ extern void tui_redisplay_readline (void); +/* Expand TABs into spaces. */ +extern char *tui_expand_tabs (const char *, int); + extern struct ui_out *tui_out; extern struct ui_out *tui_old_uiout; diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c index b523e90..961491f 100644 --- a/gdb/tui/tui-regs.c +++ b/gdb/tui/tui-regs.c @@ -36,6 +36,7 @@ #include "tui/tui-wingeneral.h" #include "tui/tui-file.h" #include "tui/tui-regs.h" +#include "tui/tui-io.h" #include "reggroups.h" #include "valprint.h" @@ -693,7 +694,9 @@ static enum tui_status tui_get_register (struct frame_info *frame, if (s && s[1] == 0) *s = 0; - ret = xstrdup (p); + /* Expand tabs into spaces, since ncurses on MS-Windows doesn't. */ + ret = tui_expand_tabs (p, 0); + do_cleanups (cleanups); return ret;