Remove fields curch and cur_line from TUI_CMD_WIN
Commit Message
These fields are currently used to track the location of the cursor
inside the command window. But their usefulness is questionable because
ncurses already internally keeps track of the location of the cursor,
whose coordinates we can query using the functions getyx(), getcurx() or
getcury(). It is an unnecessary pain to keep these fields in sync with
ncurses, and their meaning is not well-defined anyway. For instance, it
is not clear whether the coordinates held in these fields are
authoritative, or whether the coordinates reported by ncurses are.
So to keep things simple, this patch removes these fields and replaces
existing reads of these fields with calls to the appropriate ncurses
querying functions, and writes to these fields with calls to wmove()
(when necessary and applicable).
In the function tui_cont_sig(), I removed the call to wmove() entirely
because moving to (start_line, curch) makes no sense. The move should
have been to (cur_line, curch) -- which would now be a no-op.
Does this seem like a step in the right direction? These fields like an
unnecessary abstraction.
Tested on x86_64 Fedora 22, with seemingly no obvious regressions.
gdb/ChangeLog:
* tui/tui-data.h (tui_command_info): Remove fields cur_line and
curch.
* tui/tui-data.c (tui_clear_win_detail) [CMD_WIN]: Don't set
cur_line or curch, instead call wmove().
(init_win_info) [CMD_WIN]: Likewise.
* tui/tui-io.c (tui_puts): Likewise. Don't read cur_line,
instead call getcury().
(tui_redisplay_readline): Don't set cur_line or curch.
(tui_mld_erase_entire_line): Don't read cur_line, instead call
getcury().
(tui_cont_sig): Remove call to wmove.
(tui_getc): Don't read cur_line or curch, instead call getcury()
or getyx(). Don't set curch.
* tui/tui-win.c (make_visible_with_new_height) [CMD_WIN]: Don't
set cur_line or curch. Always move cursor to (0,0).
---
gdb/tui/tui-data.c | 5 +----
gdb/tui/tui-data.h | 2 --
gdb/tui/tui-io.c | 36 ++++++++++--------------------------
gdb/tui/tui-win.c | 6 +-----
4 files changed, 12 insertions(+), 37 deletions(-)
Comments
On 08/19/2015 05:38 AM, Patrick Palka wrote:
> These fields are currently used to track the location of the cursor
> inside the command window. But their usefulness is questionable because
> ncurses already internally keeps track of the location of the cursor,
> whose coordinates we can query using the functions getyx(), getcurx() or
> getcury(). It is an unnecessary pain to keep these fields in sync with
> ncurses, and their meaning is not well-defined anyway. For instance, it
> is not clear whether the coordinates held in these fields are
> authoritative, or whether the coordinates reported by ncurses are.
>
> So to keep things simple, this patch removes these fields and replaces
> existing reads of these fields with calls to the appropriate ncurses
> querying functions, and writes to these fields with calls to wmove()
> (when necessary and applicable).
>
> In the function tui_cont_sig(), I removed the call to wmove() entirely
> because moving to (start_line, curch) makes no sense. The move should
> have been to (cur_line, curch) -- which would now be a no-op.
>
> Does this seem like a step in the right direction? These fields like an
> unnecessary abstraction.
>
Sounds like a good idea to me.
> Tested on x86_64 Fedora 22, with seemingly no obvious regressions.
>
> gdb/ChangeLog:
>
> * tui/tui-data.h (tui_command_info): Remove fields cur_line and
> curch.
> * tui/tui-data.c (tui_clear_win_detail) [CMD_WIN]: Don't set
> cur_line or curch, instead call wmove().
> (init_win_info) [CMD_WIN]: Likewise.
> * tui/tui-io.c (tui_puts): Likewise. Don't read cur_line,
> instead call getcury().
> (tui_redisplay_readline): Don't set cur_line or curch.
> (tui_mld_erase_entire_line): Don't read cur_line, instead call
> getcury().
> (tui_cont_sig): Remove call to wmove.
> (tui_getc): Don't read cur_line or curch, instead call getcury()
> or getyx(). Don't set curch.
> * tui/tui-win.c (make_visible_with_new_height) [CMD_WIN]: Don't
> set cur_line or curch. Always move cursor to (0,0).
OK.
Thanks,
Pedro Alves
On Thu, Aug 20, 2015 at 1:46 PM, Pedro Alves <palves@redhat.com> wrote:
> On 08/19/2015 05:38 AM, Patrick Palka wrote:
>> These fields are currently used to track the location of the cursor
>> inside the command window. But their usefulness is questionable because
>> ncurses already internally keeps track of the location of the cursor,
>> whose coordinates we can query using the functions getyx(), getcurx() or
>> getcury(). It is an unnecessary pain to keep these fields in sync with
>> ncurses, and their meaning is not well-defined anyway. For instance, it
>> is not clear whether the coordinates held in these fields are
>> authoritative, or whether the coordinates reported by ncurses are.
>>
>> So to keep things simple, this patch removes these fields and replaces
>> existing reads of these fields with calls to the appropriate ncurses
>> querying functions, and writes to these fields with calls to wmove()
>> (when necessary and applicable).
>>
>> In the function tui_cont_sig(), I removed the call to wmove() entirely
>> because moving to (start_line, curch) makes no sense. The move should
>> have been to (cur_line, curch) -- which would now be a no-op.
>>
>> Does this seem like a step in the right direction? These fields like an
>> unnecessary abstraction.
>>
>
> Sounds like a good idea to me.
>
>> Tested on x86_64 Fedora 22, with seemingly no obvious regressions.
>>
>> gdb/ChangeLog:
>>
>> * tui/tui-data.h (tui_command_info): Remove fields cur_line and
>> curch.
>> * tui/tui-data.c (tui_clear_win_detail) [CMD_WIN]: Don't set
>> cur_line or curch, instead call wmove().
>> (init_win_info) [CMD_WIN]: Likewise.
>> * tui/tui-io.c (tui_puts): Likewise. Don't read cur_line,
>> instead call getcury().
>> (tui_redisplay_readline): Don't set cur_line or curch.
>> (tui_mld_erase_entire_line): Don't read cur_line, instead call
>> getcury().
>> (tui_cont_sig): Remove call to wmove.
>> (tui_getc): Don't read cur_line or curch, instead call getcury()
>> or getyx(). Don't set curch.
>> * tui/tui-win.c (make_visible_with_new_height) [CMD_WIN]: Don't
>> set cur_line or curch. Always move cursor to (0,0).
>
> OK.
Thanks for reviewing! Committed.
>
> Thanks,
> Pedro Alves
>
@@ -212,8 +212,7 @@ tui_clear_win_detail (struct tui_win_info *win_info)
win_info->detail.source_info.horizontal_offset = 0;
break;
case CMD_WIN:
- win_info->detail.command_info.cur_line =
- win_info->detail.command_info.curch = 0;
+ wmove (win_info->generic.handle, 0, 0);
break;
case DATA_WIN:
win_info->detail.data_display_info.data_content =
@@ -546,8 +545,6 @@ init_win_info (struct tui_win_info *win_info)
win_info->detail.data_display_info.current_group = 0;
break;
case CMD_WIN:
- win_info->detail.command_info.cur_line = 0;
- win_info->detail.command_info.curch = 0;
break;
default:
win_info->detail.opaque = NULL;
@@ -265,8 +265,6 @@ struct tui_source_info
struct tui_command_info
{
- int cur_line; /* The current line position. */
- int curch; /* The current cursor position. */
int start_line;
};
@@ -187,10 +187,7 @@ tui_puts (const char *string)
else if (c == '\n')
tui_skip_line = -1;
}
- getyx (w, TUI_CMD_WIN->detail.command_info.cur_line,
- TUI_CMD_WIN->detail.command_info.curch);
- TUI_CMD_WIN->detail.command_info.start_line
- = TUI_CMD_WIN->detail.command_info.cur_line;
+ TUI_CMD_WIN->detail.command_info.start_line = getcury (w);
}
/* Readline callback.
@@ -271,24 +268,16 @@ tui_redisplay_readline (void)
waddch (w, c);
}
if (c == '\n')
- {
- getyx (w, TUI_CMD_WIN->detail.command_info.start_line,
- TUI_CMD_WIN->detail.command_info.curch);
- }
+ TUI_CMD_WIN->detail.command_info.start_line = getcury (w);
getyx (w, line, col);
if (col < prev_col)
height++;
prev_col = col;
}
wclrtobot (w);
- getyx (w, TUI_CMD_WIN->detail.command_info.start_line,
- TUI_CMD_WIN->detail.command_info.curch);
+ TUI_CMD_WIN->detail.command_info.start_line = getcury (w);
if (c_line >= 0)
- {
- wmove (w, c_line, c_pos);
- TUI_CMD_WIN->detail.command_info.cur_line = c_line;
- TUI_CMD_WIN->detail.command_info.curch = c_pos;
- }
+ wmove (w, c_line, c_pos);
TUI_CMD_WIN->detail.command_info.start_line -= height - 1;
wrefresh (w);
@@ -371,10 +360,11 @@ static void
tui_mld_erase_entire_line (const struct match_list_displayer *displayer)
{
WINDOW *w = TUI_CMD_WIN->generic.handle;
+ int cur_y = getcury (w);
- wmove (w, TUI_CMD_WIN->detail.command_info.cur_line, 0);
+ wmove (w, cur_y, 0);
wclrtoeol (w);
- wmove (w, TUI_CMD_WIN->detail.command_info.cur_line, 0);
+ wmove (w, cur_y, 0);
}
/* TUI version of displayer.beep. */
@@ -521,10 +511,6 @@ tui_cont_sig (int sig)
/* Force a refresh of the screen. */
tui_refresh_all_win ();
- /* Update cursor position on the screen. */
- wmove (TUI_CMD_WIN->generic.handle,
- TUI_CMD_WIN->detail.command_info.start_line,
- TUI_CMD_WIN->detail.command_info.curch);
wrefresh (TUI_CMD_WIN->generic.handle);
}
signal (sig, tui_cont_sig);
@@ -601,7 +587,7 @@ tui_getc (FILE *fp)
user we recognized the command. */
if (rl_end == 0)
{
- wmove (w, TUI_CMD_WIN->detail.command_info.cur_line, 0);
+ wmove (w, getcury (w), 0);
/* Clear the line. This will blink the gdb prompt since
it will be redrawn at the same line. */
@@ -614,8 +600,8 @@ tui_getc (FILE *fp)
/* Move cursor to the end of the command line before emitting the
newline. We need to do so because when ncurses outputs a newline
it truncates any text that appears past the end of the cursor. */
- int px = TUI_CMD_WIN->detail.command_info.curch;
- int py = TUI_CMD_WIN->detail.command_info.cur_line;
+ int px, py;
+ getyx (w, py, px);
px += rl_end - rl_point;
py += px / TUI_CMD_WIN->generic.width;
px %= TUI_CMD_WIN->generic.width;
@@ -627,8 +613,6 @@ tui_getc (FILE *fp)
/* Handle prev/next/up/down here. */
ch = tui_dispatch_ctrl_char (ch);
- if (ch == '\n' || ch == '\r' || ch == '\f')
- TUI_CMD_WIN->detail.command_info.curch = 0;
if (ch == KEY_BACKSPACE)
return '\b';
@@ -1525,8 +1525,6 @@ make_visible_with_new_height (struct tui_win_info *win_info)
tui_display_all_data ();
break;
case CMD_WIN:
- win_info->detail.command_info.cur_line = 0;
- win_info->detail.command_info.curch = 0;
#ifdef HAVE_WRESIZE
wresize (TUI_CMD_WIN->generic.handle,
TUI_CMD_WIN->generic.height,
@@ -1535,9 +1533,7 @@ make_visible_with_new_height (struct tui_win_info *win_info)
mvwin (TUI_CMD_WIN->generic.handle,
TUI_CMD_WIN->generic.origin.y,
TUI_CMD_WIN->generic.origin.x);
- wmove (win_info->generic.handle,
- win_info->detail.command_info.cur_line,
- win_info->detail.command_info.curch);
+ wmove (win_info->generic.handle, 0, 0);
break;
default:
break;