Hello,
Here is a patch that I would like to propose, but it could be
controversial so I prefer to ask here first. This is in info code,
currently window->goal_column is set to -1 to mean that the
column to place the cursor in when moving it up is the column it is
currently in.
I propose in the patch to separate the information set by -1 from the
goal_column value, by setting this information in the window->flags
separately from the goal_column. This is for two reasons, first I think
that using direclty the variable value to convey another information than
the value itself (here the column index) leads to code more difficult to
understand, as when the variable value is accessed (in case of
comaprisons, in particular) or set, it may not be immediately clear if
it is to set or access the ancillary information or the main value. The
second reason is that it may be harder to debug when the value is -1 as
it could be on purpose or not. Another reason is that it is not
possible to make the variable unsigned even when it is more logical
(this is the case here, as the value is used in the LINE_MAP structure).
I would like to do similar changes in other cases if it looks like a
good idea (for node->nodestart_adjusted and node->nodelen).
Any comment?
--
Pat
diff --git a/info/session.c b/info/session.c
index 2a3ee6a325..f6158c9c47 100644
--- a/info/session.c
+++ b/info/session.c
@@ -256,7 +256,7 @@ info_read_and_dispatch (void)
means we can go from a long line to a short line and back to
a long line and end back in the same column. */
if (!(cmd == &info_next_line || cmd == &info_prev_line))
- active_window->goal_column = -1; /* Goal is current column. */
+ active_window->flags |= W_CurrentColGoal; /* Goal is current column. */
}
}
}
@@ -1261,8 +1261,11 @@ DECLARE_INFO_COMMAND (info_next_line, _("Move down to the next line"))
info_prev_line (window, -count);
else
{
- if (window->goal_column == -1)
- window->goal_column = window_get_cursor_column (window);
+ if (window->flags & W_CurrentColGoal)
+ {
+ window->goal_column = window_get_cursor_column (window);
+ window->flags &= ~W_CurrentColGoal;
+ }
while (count--)
point_next_line (window);
move_to_goal_column (window);
@@ -1276,8 +1279,11 @@ DECLARE_INFO_COMMAND (info_prev_line, _("Move up to the previous line"))
info_next_line (window, -count);
else
{
- if (window->goal_column == -1)
- window->goal_column = window_get_cursor_column (window);
+ if (window->flags & W_CurrentColGoal)
+ {
+ window->goal_column = window_get_cursor_column (window);
+ window->flags &= ~W_CurrentColGoal;
+ }
while (count--)
point_prev_line (window);
move_to_goal_column (window);
diff --git a/info/window.c b/info/window.c
index 6bbee18434..70e9ab9311 100644
--- a/info/window.c
+++ b/info/window.c
@@ -289,11 +289,11 @@ window_make_window (void)
window->height = (active_window->height / 2) - 1;
window->first_row = active_window->first_row +
(active_window->height - window->height);
- window->goal_column = -1;
+ window->goal_column = 0;
memset (&window->line_map, 0, sizeof (window->line_map));
window->modeline = xmalloc (1 + window->width);
window->line_starts = NULL;
- window->flags = W_UpdateWindow | W_WindowVisible;
+ window->flags = W_UpdateWindow | W_WindowVisible | W_CurrentColGoal;
/* Adjust the height of the old active window. */
active_window->height -= (window->height + 1);
@@ -707,7 +707,7 @@ window_unmark_chain (WINDOW *chain, int flag)
long
window_log_to_phys_line (WINDOW *window, long ln)
{
- size_t i;
+ long i;
if (ln > window->line_count)
return 0;
@@ -735,17 +735,17 @@ set_window_pagetop (WINDOW *window, int desired_top)
window->pagetop = desired_top;
/* Make sure that point appears in this window. */
+ window->goal_column = 0;
+ window->flags &= ~W_CurrentColGoal;
point_line = window_line_of_point (window);
if (point_line < window->pagetop)
{
window->point = window->line_starts[window->pagetop];
- window->goal_column = 0;
}
else if (point_line >= window->pagetop + window->height)
{
long bottom = window->pagetop + window->height - 1;
window->point = window->line_starts[bottom];
- window->goal_column = 0;
}
window->flags |= W_UpdateWindow;
diff --git a/info/window.h b/info/window.h
index f4573692d7..fc78c8db04 100644
--- a/info/window.h
+++ b/info/window.h
@@ -76,8 +76,8 @@ typedef struct window_struct
long width; /* Width of this window. */
long height; /* Height of this window. */
long first_row; /* Offset of the first line in the_screen. */
- long goal_column; /* Column to place the cursor in when moving it up and
- down. -1 means the column it is currently in. */
+ size_t goal_column; /* Column to place the cursor in when moving it up
+ and down (if W_CurrentColGoal flag is not set). */
NODE *node; /* The node displayed in this window. */
long pagetop; /* LINE_STARTS[PAGETOP] is first line in WINDOW. */
long point; /* Offset within NODE of the cursor position. */
@@ -109,6 +109,8 @@ typedef struct window_struct
#define W_NoWrap 0x10 /* Lines do not wrap in this window. */
#define W_InputWindow 0x20 /* Window accepts input. */
#define W_TempWindow 0x40 /* Window is less important. */
+#define W_CurrentColGoal 0x80 /* Use the current column to place the
+ cursor in when moving it up and down. */
extern WINDOW *windows; /* List of visible Info windows. */
extern WINDOW *active_window; /* The currently active window. */
