Message ID | 83k3149k5b.fsf@gnu.org |
---|---|
State | New, archived |
Headers |
Received: (qmail 14361 invoked by alias); 3 Jan 2015 11:30:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 14347 invoked by uid 89); 3 Jan 2015 11:30:15 -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, SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout29.012.net.il Received: from mtaout29.012.net.il (HELO mtaout29.012.net.il) (80.179.55.185) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 03 Jan 2015 11:30:13 +0000 Received: from conversion-daemon.mtaout29.012.net.il by mtaout29.012.net.il (HyperSendmail v2007.08) id <0NHL00I00LVER600@mtaout29.012.net.il> for gdb-patches@sourceware.org; Sat, 03 Jan 2015 13:27:12 +0200 (IST) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by mtaout29.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NHL00KXRMHC8F40@mtaout29.012.net.il> for gdb-patches@sourceware.org; Sat, 03 Jan 2015 13:27:12 +0200 (IST) Date: Sat, 03 Jan 2015 13:30:08 +0200 From: Eli Zaretskii <eliz@gnu.org> Subject: [PATCH] TUI: Expand TABs into spaces To: gdb-patches@sourceware.org Reply-to: Eli Zaretskii <eliz@gnu.org> Message-id: <83k3149k5b.fsf@gnu.org> X-IsSubscribed: yes |
Commit Message
Eli Zaretskii
Jan. 3, 2015, 11:30 a.m. UTC
"gdb -tui" relies on the curses library and the underlying terminal driver to expand TAB characters into spaces. But ncurses on Windows doesn't do that, and instead displays an IBM graphics character. The patches below fix that in the command window and in displaying the registers. OK to commit? 2015-01-03 Eli Zaretskii <eliz@gnu.org> * tui/tui-regs.c (tui_register_format): Expand TABs into the appropriate number of spaces. * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs into the appropriate number of spaces.
Comments
Ping! OK to install, master and 7.9 branch? > Date: Sat, 03 Jan 2015 13:30:08 +0200 > From: Eli Zaretskii <eliz@gnu.org> > > "gdb -tui" relies on the curses library and the underlying terminal > driver to expand TAB characters into spaces. But ncurses on Windows > doesn't do that, and instead displays an IBM graphics character. > > The patches below fix that in the command window and in displaying the > registers. > > OK to commit? > > 2015-01-03 Eli Zaretskii <eliz@gnu.org> > > * tui/tui-regs.c (tui_register_format): Expand TABs into the > appropriate number of spaces. > > * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs > into the appropriate number of spaces. > > > --- gdb/tui/tui-io.c~0 2014-10-29 21:45:50.000000000 +0200 > +++ gdb/tui/tui-io.c 2015-01-03 11:12:52.187500000 +0200 > @@ -179,7 +179,19 @@ tui_puts (const char *string) > else if (tui_skip_line != 1) > { > tui_skip_line = -1; > - waddch (w, c); > + 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; > @@ -254,6 +266,15 @@ tui_redisplay_readline (void) > waddch (w, '^'); > waddch (w, CTRL_CHAR (c) ? UNCTRL (c) : '?'); > } > + else if (c == '\t') > + { > + getyx (w, line, col); > + do > + { > + waddch (w, ' '); > + col++; > + } while ((col % 8) != 0); > + } > else > { > waddch (w, c); > > > --- gdb/tui/tui-regs.c~0 2014-10-29 21:45:50.000000000 +0200 > +++ gdb/tui/tui-regs.c 2015-01-03 12:52:42.062500000 +0200 > @@ -676,8 +676,9 @@ tui_register_format (struct frame_info * > struct ui_file *stream; > struct ui_file *old_stdout; > struct cleanup *cleanups; > - char *p, *s; > + char *p, *s, *q; > char *ret; > + int n_adjust, col; > > pagination_enabled = 0; > old_stdout = gdb_stdout; > @@ -694,7 +695,47 @@ tui_register_format (struct frame_info * > if (s && s[1] == 0) > *s = 0; > > - ret = xstrdup (p); > + /* Expand tabs into spaces. */ > + /* 1. How many additional characters do we need? */ > + for (col = n_adjust = 0, s = p; s; ) > + { > + s = strpbrk (s, "\t"); > + if (s) > + { > + col = (s - p) + 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 (p) + n_adjust + 1); > + > + /* 2. Copy the original string while replacing TABs with spaces. */ > + for (col = 0, s = p; 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; > + } > + > do_cleanups (cleanups); > > return ret; >
On Fri, Jan 16, 2015 at 3:17 AM, Eli Zaretskii <eliz@gnu.org> wrote: > Ping! OK to install, master and 7.9 branch? > >> Date: Sat, 03 Jan 2015 13:30:08 +0200 >> From: Eli Zaretskii <eliz@gnu.org> >> >> "gdb -tui" relies on the curses library and the underlying terminal >> driver to expand TAB characters into spaces. But ncurses on Windows >> doesn't do that, and instead displays an IBM graphics character. >> >> The patches below fix that in the command window and in displaying the >> registers. >> >> OK to commit? >> >> 2015-01-03 Eli Zaretskii <eliz@gnu.org> >> >> * tui/tui-regs.c (tui_register_format): Expand TABs into the >> appropriate number of spaces. >> >> * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs >> into the appropriate number of spaces. I'd have to read the patch more to say it's ok, but one thing that is missing are comments explaining *why* we are expanding tabs into spaces.
> Date: Fri, 16 Jan 2015 08:32:15 -0800 > From: Doug Evans <xdje42@gmail.com> > Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > > >> 2015-01-03 Eli Zaretskii <eliz@gnu.org> > >> > >> * tui/tui-regs.c (tui_register_format): Expand TABs into the > >> appropriate number of spaces. > >> > >> * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs > >> into the appropriate number of spaces. > > I'd have to read the patch more to say it's ok, > but one thing that is missing are comments > explaining *why* we are expanding tabs into spaces. You mean, the fact that the Windows port of ncurses doesn't? Sure, can do that. But note that a similar feature in displaying the source doesn't have any such comments.
On Fri, Jan 16, 2015 at 8:43 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> Date: Fri, 16 Jan 2015 08:32:15 -0800 >> From: Doug Evans <xdje42@gmail.com> >> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> >> >> >> 2015-01-03 Eli Zaretskii <eliz@gnu.org> >> >> >> >> * tui/tui-regs.c (tui_register_format): Expand TABs into the >> >> appropriate number of spaces. >> >> >> >> * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs >> >> into the appropriate number of spaces. >> >> I'd have to read the patch more to say it's ok, >> but one thing that is missing are comments >> explaining *why* we are expanding tabs into spaces. > > You mean, the fact that the Windows port of ncurses doesn't? Sure, > can do that. But note that a similar feature in displaying the source > doesn't have any such comments. Is that in relation to the tabset command? And is that to provide a knob so that source written with an expectation that \t was different than 8 can be made readable? Or is there existing code that is coping with these windows, umm, quirks?
> Date: Fri, 16 Jan 2015 09:29:52 -0800 > From: Doug Evans <xdje42@gmail.com> > Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > > Is that in relation to the tabset command? > And is that to provide a knob so that source written > with an expectation that \t was different than 8 > can be made readable? No. > Or is there existing code that is coping with these > windows, umm, quirks? Yes.
On Fri, Jan 16, 2015 at 9:53 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> Date: Fri, 16 Jan 2015 09:29:52 -0800 >> From: Doug Evans <xdje42@gmail.com> >> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> >> >> Is that in relation to the tabset command? >> And is that to provide a knob so that source written >> with an expectation that \t was different than 8 >> can be made readable? > > No. > >> Or is there existing code that is coping with these >> windows, umm, quirks? > > Yes. Good to know. Can you point me at it?
> Date: Fri, 16 Jan 2015 10:25:10 -0800 > From: Doug Evans <xdje42@gmail.com> > Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > > >> Or is there existing code that is coping with these > >> windows, umm, quirks? > > > > Yes. > > Good to know. > Can you point me at it? Point you at what?
On Fri, Jan 16, 2015 at 12:11 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> Date: Fri, 16 Jan 2015 10:25:10 -0800 >> From: Doug Evans <xdje42@gmail.com> >> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> >> >> >> Or is there existing code that is coping with these >> >> windows, umm, quirks? >> > >> > Yes. >> >> Good to know. >> Can you point me at it? > > Point you at what? The "similar feature in displaying the source doesn't have any such comments." I thought I found it, but then wasn't sure, and I don't want to guess.
> Date: Fri, 16 Jan 2015 17:02:27 -0800 > From: Doug Evans <xdje42@gmail.com> > Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > > >> Can you point me at it? > > > > Point you at what? > > The "similar feature in displaying the source > doesn't have any such comments." Ah, okay. tui-source.c:tui_set_source_content, around line 185 of tui-source.c, is what I had in mind.
Ping! Ping! OK to install? > Date: Fri, 16 Jan 2015 08:32:15 -0800 > From: Doug Evans <xdje42@gmail.com> > Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > > On Fri, Jan 16, 2015 at 3:17 AM, Eli Zaretskii <eliz@gnu.org> wrote: > > Ping! OK to install, master and 7.9 branch? > > > >> Date: Sat, 03 Jan 2015 13:30:08 +0200 > >> From: Eli Zaretskii <eliz@gnu.org> > >> > >> "gdb -tui" relies on the curses library and the underlying terminal > >> driver to expand TAB characters into spaces. But ncurses on Windows > >> doesn't do that, and instead displays an IBM graphics character. > >> > >> The patches below fix that in the command window and in displaying the > >> registers. > >> > >> OK to commit? > >> > >> 2015-01-03 Eli Zaretskii <eliz@gnu.org> > >> > >> * tui/tui-regs.c (tui_register_format): Expand TABs into the > >> appropriate number of spaces. > >> > >> * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs > >> into the appropriate number of spaces. > > I'd have to read the patch more to say it's ok, > but one thing that is missing are comments > explaining *why* we are expanding tabs into spaces. >
Eli Zaretskii <eliz@gnu.org> writes: > Ping! Ping! OK to install? "Give me a ping, Vasili. One ping only, please." ref: http://www.imdb.com/title/tt0099810/quotes [just a little light humor] >> Date: Fri, 16 Jan 2015 08:32:15 -0800 >> From: Doug Evans <xdje42@gmail.com> >> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> >> >> On Fri, Jan 16, 2015 at 3:17 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> > Ping! OK to install, master and 7.9 branch? >> > >> >> Date: Sat, 03 Jan 2015 13:30:08 +0200 >> >> From: Eli Zaretskii <eliz@gnu.org> >> >> >> >> "gdb -tui" relies on the curses library and the underlying terminal >> >> driver to expand TAB characters into spaces. But ncurses on Windows >> >> doesn't do that, and instead displays an IBM graphics character. >> >> >> >> The patches below fix that in the command window and in displaying the >> >> registers. >> >> >> >> OK to commit? >> >> >> >> 2015-01-03 Eli Zaretskii <eliz@gnu.org> >> >> >> >> * tui/tui-regs.c (tui_register_format): Expand TABs into the >> >> appropriate number of spaces. >> >> >> >> * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs >> >> into the appropriate number of spaces. >> >> I'd have to read the patch more to say it's ok, >> but one thing that is missing are comments >> explaining *why* we are expanding tabs into spaces. I check the code (tui-source.c around line 185) and it has this: else { /* Store the charcter in the line buffer. If it is a tab, then translate to the correct number of chars so we don't overwrite our buffer. */ So it *does* have a comment explaining why tabs are being expanded. For tui_puts and tui_redisplay_readline let's add something like the following: /* Windows ncurses doesn't expand tabs, so we have to do that here. */ [else] if (c == '\t') For tui_register_format it used to be such a straightforward function, it's a shame to grow it by 2x for bitfiddly tab handling. Let's add a helper routine that takes one string and returns another with tabs expanded, and put this helper routine in, say tui-io.c (this routine isn't tui-regs specific), and then call that routine from tui_register_format. I didn't review the actual code to do the tab expansion with a microscope. I'm going to assume it at least mostly works. At least it'll be tucked away. Also, a note on the ChangeLog: No blank lines between related entries. > * tui/tui-regs.c (tui_register_format): Expand TABs into the > appropriate number of spaces. > > * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs > into the appropriate number of spaces.
--- gdb/tui/tui-io.c~0 2014-10-29 21:45:50.000000000 +0200 +++ gdb/tui/tui-io.c 2015-01-03 11:12:52.187500000 +0200 @@ -179,7 +179,19 @@ tui_puts (const char *string) else if (tui_skip_line != 1) { tui_skip_line = -1; - waddch (w, c); + 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; @@ -254,6 +266,15 @@ tui_redisplay_readline (void) waddch (w, '^'); waddch (w, CTRL_CHAR (c) ? UNCTRL (c) : '?'); } + else if (c == '\t') + { + getyx (w, line, col); + do + { + waddch (w, ' '); + col++; + } while ((col % 8) != 0); + } else { waddch (w, c); --- gdb/tui/tui-regs.c~0 2014-10-29 21:45:50.000000000 +0200 +++ gdb/tui/tui-regs.c 2015-01-03 12:52:42.062500000 +0200 @@ -676,8 +676,9 @@ tui_register_format (struct frame_info * struct ui_file *stream; struct ui_file *old_stdout; struct cleanup *cleanups; - char *p, *s; + char *p, *s, *q; char *ret; + int n_adjust, col; pagination_enabled = 0; old_stdout = gdb_stdout; @@ -694,7 +695,47 @@ tui_register_format (struct frame_info * if (s && s[1] == 0) *s = 0; - ret = xstrdup (p); + /* Expand tabs into spaces. */ + /* 1. How many additional characters do we need? */ + for (col = n_adjust = 0, s = p; s; ) + { + s = strpbrk (s, "\t"); + if (s) + { + col = (s - p) + 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 (p) + n_adjust + 1); + + /* 2. Copy the original string while replacing TABs with spaces. */ + for (col = 0, s = p; 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; + } + do_cleanups (cleanups); return ret;