* [RFC] Getting rid of selected_window
@ 2013-11-29 11:53 Dmitry Antipov
2013-11-29 14:38 ` Stefan Monnier
2013-11-29 15:19 ` Eli Zaretskii
0 siblings, 2 replies; 7+ messages in thread
From: Dmitry Antipov @ 2013-11-29 11:53 UTC (permalink / raw)
To: Emacs development discussions
[-- Attachment #1: Type: text/plain, Size: 247 bytes --]
I'm trying to get rid of selected_window, with an intent to completely cleanup corner
cases where FRAME_SELECTED_WINDOW (selected_frame) is not the same as selected_window
and avoid redundant synchronization of them. Comments are welcome.
Dmitry
[-- Attachment #2: drop_selected_window.patch --]
[-- Type: text/x-patch, Size: 41149 bytes --]
=== modified file 'src/buffer.c'
--- src/buffer.c 2013-11-28 22:43:09 +0000
+++ src/buffer.c 2013-11-29 11:02:47 +0000
@@ -2402,7 +2402,7 @@
live window points to that window's buffer. So since we
just swapped the markers between the two buffers, we need
to undo the effect of this swap for window markers. */
- Lisp_Object w = selected_window, ws = Qnil;
+ Lisp_Object w = Fselected_window (), ws = Qnil;
Lisp_Object buf1, buf2;
XSETBUFFER (buf1, current_buffer); XSETBUFFER (buf2, other_buffer);
=== modified file 'src/callint.c'
--- src/callint.c 2013-09-04 20:22:37 +0000
+++ src/callint.c 2013-11-29 11:02:47 +0000
@@ -509,7 +509,7 @@
case 'b': /* Name of existing buffer. */
args[i] = Fcurrent_buffer ();
- if (EQ (selected_window, minibuf_window))
+ if (EQ (Fselected_window (), minibuf_window))
args[i] = Fother_buffer (args[i], Qnil, Qnil);
args[i] = Fread_buffer (callint_message, args[i], Qt);
break;
=== modified file 'src/cmds.c'
--- src/cmds.c 2013-09-05 02:27:13 +0000
+++ src/cmds.c 2013-11-29 11:02:47 +0000
@@ -282,7 +282,7 @@
nonundocount = 0;
if (NILP (Vexecuting_kbd_macro)
- && !EQ (minibuf_window, selected_window))
+ && !EQ (minibuf_window, Fselected_window ()))
{
if (nonundocount <= 0 || nonundocount >= 20)
{
=== modified file 'src/dispextern.h'
--- src/dispextern.h 2013-11-06 00:14:56 +0000
+++ src/dispextern.h 2013-11-29 11:02:47 +0000
@@ -1410,7 +1410,7 @@
#define CURRENT_MODE_LINE_FACE_ID_3(SELW, MBW, SCRW) \
((!mode_line_in_non_selected_windows \
- || (SELW) == XWINDOW (selected_window) \
+ || (SELW) == SELECTED_WINDOW () \
|| (minibuf_level > 0 \
&& !NILP (minibuf_selected_window) \
&& (MBW) == XWINDOW (minibuf_window) \
@@ -1422,7 +1422,7 @@
/* Return the desired face id for the mode line of window W. */
#define CURRENT_MODE_LINE_FACE_ID(W) \
- (CURRENT_MODE_LINE_FACE_ID_3((W), XWINDOW (selected_window), (W)))
+ (CURRENT_MODE_LINE_FACE_ID_3((W), SELECTED_WINDOW (), (W)))
/* Return the current height of the mode line of window W. If not known
from W->mode_line_height, look at W's current glyph matrix, or return
=== modified file 'src/dispnew.c'
--- src/dispnew.c 2013-11-06 04:11:04 +0000
+++ src/dispnew.c 2013-11-29 11:02:47 +0000
@@ -4483,7 +4483,7 @@
/* If we are showing a message instead of the mini-buffer,
show the cursor for the message instead of for the
(now hidden) mini-buffer contents. */
- || (EQ (minibuf_window, selected_window)
+ || (EQ (minibuf_window, Fselected_window ())
&& EQ (minibuf_window, echo_area_window)
&& !NILP (echo_area_buffer[0])))
/* These cases apply only to the frame that contains
=== modified file 'src/editfns.c'
--- src/editfns.c 2013-11-29 01:22:40 +0000
+++ src/editfns.c 2013-11-29 11:02:47 +0000
@@ -832,8 +832,8 @@
? Fcopy_marker (BVAR (current_buffer, mark), Qnil)
: Qnil),
/* Selected window if current buffer is shown in it, nil otherwise. */
- (EQ (XWINDOW (selected_window)->contents, Fcurrent_buffer ())
- ? selected_window : Qnil),
+ (EQ (SELECTED_WINDOW ()->contents, Fcurrent_buffer ())
+ ? Fselected_window () : Qnil),
BVAR (current_buffer, mark_active));
}
@@ -900,7 +900,7 @@
buffer, restore point in that window. */
tem = XSAVE_OBJECT (info, 2);
if (WINDOWP (tem)
- && !EQ (tem, selected_window)
+ && !EQ (tem, Fselected_window ())
&& (tem1 = XWINDOW (tem)->contents,
(/* Window is live... */
BUFFERP (tem1)
=== modified file 'src/fileio.c'
--- src/fileio.c 2013-11-27 16:08:53 +0000
+++ src/fileio.c 2013-11-29 11:13:44 +0000
@@ -3868,8 +3868,8 @@
/* If display currently starts at beginning of line,
keep it that way. */
- if (XBUFFER (XWINDOW (selected_window)->contents) == current_buffer)
- XWINDOW (selected_window)->start_at_line_beg = !NILP (Fbolp ());
+ if (SELECTED_BUFFER () == current_buffer)
+ SELECTED_WINDOW ()->start_at_line_beg = !NILP (Fbolp ());
replace_handled = 1;
}
@@ -4014,8 +4014,8 @@
/* If display currently starts at beginning of line,
keep it that way. */
- if (XBUFFER (XWINDOW (selected_window)->contents) == current_buffer)
- XWINDOW (selected_window)->start_at_line_beg = !NILP (Fbolp ());
+ if (SELECTED_BUFFER () == current_buffer)
+ SELECTED_WINDOW ()->start_at_line_beg = !NILP (Fbolp ());
/* Replace the chars that we need to replace,
and update INSERTED to equal the number of bytes
=== modified file 'src/frame.c'
--- src/frame.c 2013-11-28 22:43:09 +0000
+++ src/frame.c 2013-11-29 11:02:47 +0000
@@ -1245,7 +1245,7 @@
/* At this point, we are committed to deleting the frame.
There is no more chance for errors to prevent it. */
- minibuffer_selected = EQ (minibuf_window, selected_window);
+ minibuffer_selected = EQ (minibuf_window, Fselected_window ());
sf = SELECTED_FRAME ();
/* Don't let the frame remain selected. */
if (f == sf)
@@ -1727,7 +1727,7 @@
error ("Attempt to make invisible the sole visible or iconified frame");
/* Don't allow minibuf_window to remain on an invisible frame. */
- check_minibuf_window (frame, EQ (minibuf_window, selected_window));
+ check_minibuf_window (frame, EQ (minibuf_window, Fselected_window ()));
/* I think this should be done with a hook. */
#ifdef HAVE_WINDOW_SYSTEM
@@ -1750,7 +1750,7 @@
struct frame *f = decode_live_frame (frame);
/* Don't allow minibuf_window to remain on an iconified frame. */
- check_minibuf_window (frame, EQ (minibuf_window, selected_window));
+ check_minibuf_window (frame, EQ (minibuf_window, Fselected_window ()));
/* I think this should be done with a hook. */
#ifdef HAVE_WINDOW_SYSTEM
=== modified file 'src/fringe.c'
--- src/fringe.c 2013-10-26 03:13:18 +0000
+++ src/fringe.c 2013-11-29 11:02:47 +0000
@@ -1745,7 +1745,7 @@
args_out_of_range (window, pos);
textpos = XINT (pos);
}
- else if (w == XWINDOW (selected_window))
+ else if (w == SELECTED_WINDOW ())
textpos = PT;
else
textpos = marker_position (w->pointm);
=== modified file 'src/keyboard.c'
--- src/keyboard.c 2013-11-29 01:22:40 +0000
+++ src/keyboard.c 2013-11-29 11:14:33 +0000
@@ -823,8 +823,7 @@
command_loop_level++;
update_mode_lines = 17;
- if (command_loop_level
- && current_buffer != XBUFFER (XWINDOW (selected_window)->contents))
+ if (command_loop_level && current_buffer != SELECTED_BUFFER ())
buffer = Fcurrent_buffer ();
else
buffer = Qnil;
@@ -1381,7 +1380,7 @@
Fkill_emacs (Qnil);
/* Make sure the current window's buffer is selected. */
- set_buffer_internal (XBUFFER (XWINDOW (selected_window)->contents));
+ set_buffer_internal (SELECTED_BUFFER ());
/* Display any malloc warning that just came out. Use while because
displaying one warning can cause another. */
@@ -1447,7 +1446,7 @@
/* A filter may have run while we were reading the input. */
if (! FRAME_LIVE_P (XFRAME (selected_frame)))
Fkill_emacs (Qnil);
- set_buffer_internal (XBUFFER (XWINDOW (selected_window)->contents));
+ set_buffer_internal (SELECTED_BUFFER ());
++num_input_keys;
@@ -1474,11 +1473,11 @@
from that position. But also throw away beg_unchanged and
end_unchanged information in that case, so that redisplay will
update the whole window properly. */
- if (XWINDOW (selected_window)->force_start)
+ if (SELECTED_WINDOW ()->force_start)
{
struct buffer *b;
- XWINDOW (selected_window)->force_start = 0;
- b = XBUFFER (XWINDOW (selected_window)->contents);
+ SELECTED_WINDOW ()->force_start = 0;
+ b = SELECTED_BUFFER ();
BUF_BEG_UNCHANGED (b) = BUF_END_UNCHANGED (b) = 0;
}
@@ -2559,7 +2558,7 @@
/* Redisplay if no pending input. */
while (!input_pending)
{
- if (help_echo_showing_p && !EQ (selected_window, minibuf_window))
+ if (help_echo_showing_p && !EQ (Fselected_window (), minibuf_window))
redisplay_preserve_echo_area (5);
else
redisplay ();
@@ -2778,7 +2777,7 @@
/* Slow down auto saves logarithmically in size of current buffer,
and garbage collect while we're at it. */
- if (! MINI_WINDOW_P (XWINDOW (selected_window)))
+ if (! MINI_WINDOW_P (SELECTED_WINDOW ()))
last_non_minibuf_size = Z - BEG;
buffer_size = (last_non_minibuf_size >> 8) + 1;
delay_level = 0;
@@ -5250,7 +5249,7 @@
&object, &dx, &dy, &width, &height);
if (STRINGP (string))
string_info = Fcons (string, make_number (charpos));
- textpos = (w == XWINDOW (selected_window)
+ textpos = (w == SELECTED_WINDOW ()
&& current_buffer == XBUFFER (w->contents))
? PT : marker_position (w->pointm);
@@ -9152,9 +9151,8 @@
{
if (! FRAME_LIVE_P (XFRAME (selected_frame)))
Fkill_emacs (Qnil);
- if (XBUFFER (XWINDOW (selected_window)->contents)
- != current_buffer)
- Fset_buffer (XWINDOW (selected_window)->contents);
+ if (SELECTED_BUFFER () != current_buffer)
+ Fset_buffer (SELECTED_WINDOW ()->contents);
}
goto replay_sequence;
@@ -9201,10 +9199,8 @@
read_key_sequence, and a timer (or process-filter or
special-event-map, ...) might have switched the current buffer
or the selected window from under us in the mean time. */
- if (fix_current_buffer
- && (XBUFFER (XWINDOW (selected_window)->contents)
- != current_buffer))
- Fset_buffer (XWINDOW (selected_window)->contents);
+ if (fix_current_buffer && (SELECTED_BUFFER () != current_buffer))
+ Fset_buffer (SELECTED_WINDOW ()->contents);
current_binding = active_maps (first_event);
}
@@ -10673,7 +10669,7 @@
CHECK_NATNUM (y);
if (NILP (frame_or_window))
- frame_or_window = selected_window;
+ frame_or_window = Fselected_window ();
if (WINDOWP (frame_or_window))
{
@@ -10709,7 +10705,7 @@
Lisp_Object tem;
if (NILP (window))
- window = selected_window;
+ window = Fselected_window ();
tem = Fpos_visible_in_window_p (pos, window, Qt);
if (!NILP (tem))
=== modified file 'src/menu.c'
--- src/menu.c 2013-11-29 05:25:25 +0000
+++ src/menu.c 2013-11-29 11:02:47 +0000
@@ -1271,7 +1271,7 @@
XSETFRAME (window, new_f);
else
{
- window = selected_window;
+ window = Fselected_window ();
XSETFASTINT (x, 0);
XSETFASTINT (y, 0);
}
@@ -1511,7 +1511,7 @@
else
window = selected_window;
#endif
- window = selected_window;
+ window = Fselected_window ();
}
else if (CONSP (position))
{
=== modified file 'src/minibuf.c'
--- src/minibuf.c 2013-11-28 22:43:09 +0000
+++ src/minibuf.c 2013-11-29 11:02:47 +0000
@@ -441,7 +441,7 @@
if (!enable_recursive_minibuffers
&& minibuf_level > 0)
{
- if (EQ (selected_window, minibuf_window))
+ if (EQ (Fselected_window (), minibuf_window))
error ("Command attempted to use minibuffer while in minibuffer");
else
/* If we're in another window, cancel the minibuffer that's active. */
@@ -582,9 +582,9 @@
if (!EQ (mini_frame, selected_frame))
Fredirect_frame_focus (selected_frame, mini_frame);
- Vminibuf_scroll_window = selected_window;
- if (minibuf_level == 1 || !EQ (minibuf_window, selected_window))
- minibuf_selected_window = selected_window;
+ Vminibuf_scroll_window = Fselected_window ();
+ if (minibuf_level == 1 || !EQ (minibuf_window, Fselected_window ()))
+ minibuf_selected_window = Fselected_window ();
/* Empty out the minibuffers of all frames other than the one
where we are going to display one now.
=== modified file 'src/process.c'
--- src/process.c 2013-11-23 02:58:28 +0000
+++ src/process.c 2013-11-29 11:02:47 +0000
@@ -4361,7 +4361,7 @@
{
unsigned old_timers_run = timers_run;
struct buffer *old_buffer = current_buffer;
- Lisp_Object old_window = selected_window;
+ Lisp_Object old_window = Fselected_window ();
timer_delay = timer_check ();
@@ -4369,7 +4369,7 @@
an alike. Make read_key_sequence aware of that. */
if (timers_run != old_timers_run
&& (old_buffer != current_buffer
- || !EQ (old_window, selected_window))
+ || !EQ (old_window, Fselected_window ()))
&& waiting_for_user_input_p == -1)
record_asynch_buffer_change ();
@@ -4674,7 +4674,7 @@
{
unsigned old_timers_run = timers_run;
struct buffer *old_buffer = current_buffer;
- Lisp_Object old_window = selected_window;
+ Lisp_Object old_window = Fselected_window ();
bool leave = 0;
if (detect_input_pending_run_timers (do_display))
@@ -4689,7 +4689,7 @@
if (timers_run != old_timers_run
&& waiting_for_user_input_p == -1
&& (old_buffer != current_buffer
- || !EQ (old_window, selected_window)))
+ || !EQ (old_window, Fselected_window ())))
record_asynch_buffer_change ();
if (leave)
=== modified file 'src/term.c'
--- src/term.c 2013-11-29 01:22:40 +0000
+++ src/term.c 2013-11-29 11:02:47 +0000
@@ -216,7 +216,7 @@
{
struct tty_display_info *tty = FRAME_TTY (f);
- if (!XWINDOW (selected_window)->cursor_off_p)
+ if (!SELECTED_WINDOW ()->cursor_off_p)
tty_show_cursor (tty);
tty_turn_off_insert (tty);
tty_background_highlight (tty);
=== modified file 'src/window.c'
--- src/window.c 2013-11-29 01:22:40 +0000
+++ src/window.c 2013-11-29 11:24:03 +0000
@@ -85,15 +85,6 @@
Lisp_Object, Lisp_Object);
static void apply_window_adjustment (struct window *);
-/* This is the window in which the terminal's cursor should
- be left when nothing is being done with it. This must
- always be a leaf window, and its buffer is selected by
- the top level editing loop at the end of each command.
-
- This value is always the same as
- FRAME_SELECTED_WINDOW (selected_frame). */
-Lisp_Object selected_window;
-
/* A list of all windows for use by next_window and Fwindow_list.
Functions creating or deleting windows should invalidate this cache
by setting it to nil. */
@@ -226,7 +217,7 @@
decode_live_window (register Lisp_Object window)
{
if (NILP (window))
- return XWINDOW (selected_window);
+ return SELECTED_WINDOW ();
CHECK_LIVE_WINDOW (window);
return XWINDOW (window);
@@ -238,7 +229,7 @@
struct window *w;
if (NILP (window))
- return XWINDOW (selected_window);
+ return SELECTED_WINDOW ();
CHECK_WINDOW (window);
w = XWINDOW (window);
@@ -251,7 +242,7 @@
struct window *w;
if (NILP (window))
- return XWINDOW (selected_window);
+ return SELECTED_WINDOW ();
CHECK_VALID_WINDOW (window);
w = XWINDOW (window);
@@ -452,7 +443,8 @@
selected windows appears and to which many commands apply. */)
(void)
{
- return selected_window;
+ /* This is expected to be called when selected_frame is initialized. */
+ return FRAME_SELECTED_WINDOW (SELECTED_FRAME ());
}
int window_select_count;
@@ -475,7 +467,7 @@
/* Make the selected window's buffer current. */
Fset_buffer (w->contents);
- if (EQ (window, selected_window) && !inhibit_point_swap)
+ if (EQ (window, Fselected_window ()) && !inhibit_point_swap)
/* `switch-to-buffer' uses (select-window (selected-window)) as a "clever"
way to call record_buffer from Elisp, so it's important that we call
record_buffer before returning here. */
@@ -484,7 +476,7 @@
if (NILP (norecord))
/* Mark the window for redisplay since the selected-window has a different
mode-line. */
- wset_redisplay (XWINDOW (selected_window));
+ wset_redisplay (SELECTED_WINDOW ());
else
redisplay_other_windows ();
sf = SELECTED_FRAME ();
@@ -497,7 +489,7 @@
frame is active. */
Fselect_frame (WINDOW_FRAME (w), norecord);
/* Fselect_frame called us back so we've done all the work already. */
- eassert (EQ (window, selected_window));
+ eassert (EQ (window, Fselected_window ()));
return window;
}
else
@@ -532,14 +524,14 @@
not selected, must be in the window. */
if (!inhibit_point_swap)
{
- struct window *ow = XWINDOW (selected_window);
+ struct window *ow = SELECTED_WINDOW ();
if (BUFFERP (ow->contents))
set_marker_both (ow->pointm, ow->contents,
BUF_PT (XBUFFER (ow->contents)),
BUF_PT_BYTE (XBUFFER (ow->contents)));
}
- selected_window = window;
+ fset_selected_window (SELECTED_FRAME (), window);
/* Go to the point recorded in the window.
This is important when the buffer is in more
@@ -1437,7 +1429,7 @@
{
register struct window *w = decode_live_window (window);
- if (w == XWINDOW (selected_window))
+ if (w == SELECTED_WINDOW ())
return make_number (BUF_PT (XBUFFER (w->contents)));
else
return Fmarker_position (w->pointm);
@@ -1539,7 +1531,7 @@
/* Type of POS is checked by Fgoto_char or set_marker_restricted ... */
- if (w == XWINDOW (selected_window))
+ if (w == SELECTED_WINDOW ())
{
if (XBUFFER (w->contents) == current_buffer)
Fgoto_char (pos);
@@ -1582,7 +1574,7 @@
w->update_mode_line = 1;
/* Bug#15957. */
w->window_end_valid = 0;
- if (w != XWINDOW (selected_window))
+ if (w != SELECTED_WINDOW ())
wset_redisplay (w);
return pos;
@@ -1629,7 +1621,7 @@
CHECK_NUMBER_COERCE_MARKER (pos);
posint = XINT (pos);
}
- else if (w == XWINDOW (selected_window))
+ else if (w == SELECTED_WINDOW ())
posint = PT;
else
posint = marker_position (w->pointm);
@@ -1965,7 +1957,7 @@
/* Point in the selected window's buffer
is actually stored in that buffer, and the window's pointm isn't used.
So don't clobber point in that buffer. */
- if (! EQ (buf, XWINDOW (selected_window)->contents)
+ if (! EQ (buf, SELECTED_WINDOW ()->contents)
/* Don't clobber point in current buffer either (this could be
useful in connection with bug#12208).
&& XBUFFER (buf) != current_buffer */
@@ -2473,7 +2465,8 @@
(Lisp_Object frame, Lisp_Object minibuf, Lisp_Object window)
{
if (NILP (window))
- window = FRAMEP (frame) ? XFRAME (frame)->selected_window : selected_window;
+ window = (FRAMEP (frame) ? XFRAME (frame)->selected_window
+ : Fselected_window ());
CHECK_WINDOW (window);
if (NILP (frame))
frame = selected_frame;
@@ -2604,7 +2597,7 @@
is currently in use. */
&& (MINI_WINDOW_P (w) ? EQ (window, minibuf_window) : 1))
{
- if (EQ (window, selected_window))
+ if (EQ (window, Fselected_window ()))
/* Preferably return the selected window. */
RETURN_UNGCPRO (window);
else if (EQ (XWINDOW (window)->frame, selected_frame)
@@ -2634,7 +2627,7 @@
/* If WINDOW is the selected window, make its buffer
current. But do so only if the window shows the
current buffer (Bug#6454). */
- if (EQ (window, selected_window)
+ if (EQ (window, Fselected_window ())
&& XBUFFER (w->contents) == current_buffer)
Fset_buffer (w->contents);
}
@@ -3103,7 +3096,7 @@
buffer)))
{
ptrdiff_t inner_count = SPECPDL_INDEX ();
- record_unwind_protect (select_window_norecord, selected_window);
+ record_unwind_protect (select_window_norecord, Fselected_window ());
select_window_norecord (window);
run_funs (Fbuffer_local_value (Qwindow_configuration_change_hook,
buffer));
@@ -3143,7 +3136,7 @@
wset_buffer (w, buffer);
- if (EQ (window, selected_window))
+ if (w == SELECTED_WINDOW ())
bset_last_selected_window (b, window);
/* Let redisplay errors through. */
@@ -3353,7 +3346,7 @@
{
ptrdiff_t count = SPECPDL_INDEX ();
Lisp_Object prev_window, prev_buffer;
- prev_window = selected_window;
+ prev_window = Fselected_window ();
XSETBUFFER (prev_buffer, old);
/* Select the window that was chosen, for running the hook.
@@ -4070,7 +4063,7 @@
/* `get-mru-window' might fail for some reason so play it safe
- promote the first window _without recording it_ first. */
- if (EQ (FRAME_SELECTED_WINDOW (f), selected_window))
+ if (EQ (FRAME_SELECTED_WINDOW (f), Fselected_window ()))
Fselect_window (new_selected_window, Qt);
else
fset_selected_window (f, new_selected_window);
@@ -4084,7 +4077,7 @@
new_selected_window = mru_window;
/* If all ended up well, we now promote the mru window. */
- if (EQ (FRAME_SELECTED_WINDOW (f), selected_window))
+ if (EQ (FRAME_SELECTED_WINDOW (f), Fselected_window ()))
Fselect_window (new_selected_window, Qnil);
else
fset_selected_window (f, new_selected_window);
@@ -4838,23 +4831,23 @@
/* If selected window's buffer isn't current, make it current for
the moment. But don't screw up if window_scroll gets an error. */
- if (XBUFFER (XWINDOW (selected_window)->contents) != current_buffer)
+ if (SELECTED_BUFFER () != current_buffer)
{
record_unwind_protect (save_excursion_restore, save_excursion_save ());
- Fset_buffer (XWINDOW (selected_window)->contents);
+ Fset_buffer (SELECTED_WINDOW ()->contents);
/* Make redisplay consider other windows than just selected_window. */
windows_or_buffers_changed = 37;
}
if (NILP (n))
- window_scroll (selected_window, direction, 1, 0);
+ window_scroll (Fselected_window (), direction, 1, 0);
else if (EQ (n, Qminus))
- window_scroll (selected_window, -direction, 1, 0);
+ window_scroll (Fselected_window (), -direction, 1, 0);
else
{
n = Fprefix_numeric_value (n);
- window_scroll (selected_window, XINT (n) * direction, 0, 0);
+ window_scroll (Fselected_window (), XINT (n) * direction, 0, 0);
}
unbind_to (count, Qnil);
@@ -4897,7 +4890,7 @@
{
Lisp_Object window;
- if (MINI_WINDOW_P (XWINDOW (selected_window))
+ if (MINI_WINDOW_P (SELECTED_WINDOW ())
&& !NILP (Vminibuf_scroll_window))
window = Vminibuf_scroll_window;
/* If buffer is specified, scroll that buffer. */
@@ -4911,20 +4904,20 @@
{
/* Nothing specified; look for a neighboring window on the same
frame. */
- window = Fnext_window (selected_window, Qnil, Qnil);
+ window = Fnext_window (Fselected_window (), Qnil, Qnil);
- if (EQ (window, selected_window))
+ if (EQ (window, Fselected_window ()))
/* That didn't get us anywhere; look for a window on another
visible frame. */
do
window = Fnext_window (window, Qnil, Qt);
while (! FRAME_VISIBLE_P (XFRAME (WINDOW_FRAME (XWINDOW (window))))
- && ! EQ (window, selected_window));
+ && ! EQ (window, Fselected_window ()));
}
CHECK_LIVE_WINDOW (window);
- if (EQ (window, selected_window))
+ if (EQ (window, Fselected_window ()))
error ("There is no other window");
return window;
@@ -4988,7 +4981,7 @@
by this function. This happens in an interactive call. */)
(register Lisp_Object arg, Lisp_Object set_minimum)
{
- struct window *w = XWINDOW (selected_window);
+ struct window *w = SELECTED_WINDOW ();
EMACS_INT requested_arg = (NILP (arg)
? window_body_cols (w) - 2
: XINT (Fprefix_numeric_value (arg)));
@@ -5011,7 +5004,7 @@
by this function. This happens in an interactive call. */)
(register Lisp_Object arg, Lisp_Object set_minimum)
{
- struct window *w = XWINDOW (selected_window);
+ struct window *w = SELECTED_WINDOW ();
EMACS_INT requested_arg = (NILP (arg)
? window_body_cols (w) - 2
: XINT (Fprefix_numeric_value (arg)));
@@ -5029,7 +5022,7 @@
(void)
{
if (minibuf_level > 0
- && MINI_WINDOW_P (XWINDOW (selected_window))
+ && MINI_WINDOW_P (SELECTED_WINDOW ())
&& WINDOW_LIVE_P (minibuf_selected_window))
return minibuf_selected_window;
@@ -5108,7 +5101,7 @@
and redisplay normally--don't erase and redraw the frame. */)
(register Lisp_Object arg)
{
- struct window *w = XWINDOW (selected_window);
+ struct window *w = SELECTED_WINDOW ();
struct buffer *buf = XBUFFER (w->contents);
struct buffer *obuf = current_buffer;
bool center_p = 0;
@@ -5312,7 +5305,7 @@
zero means top of window, negative means relative to bottom of window. */)
(Lisp_Object arg)
{
- struct window *w = XWINDOW (selected_window);
+ struct window *w = SELECTED_WINDOW ();
int lines, start;
Lisp_Object window;
#if 0
@@ -5324,7 +5317,7 @@
when passed below to set_marker_both. */
error ("move-to-window-line called from unrelated buffer");
- window = selected_window;
+ window = Fselected_window ();
start = marker_position (w->start);
if (start < BEGV || start > ZV)
{
@@ -5481,9 +5474,8 @@
the current-selected-window. So we have to be careful which
point of the current-buffer we copy into old_point. */
if (EQ (XWINDOW (data->current_window)->contents, new_current_buffer)
- && WINDOWP (selected_window)
- && EQ (XWINDOW (selected_window)->contents, new_current_buffer)
- && !EQ (selected_window, data->current_window))
+ && EQ (SELECTED_WINDOW ()->contents, new_current_buffer)
+ && !EQ (Fselected_window (), data->current_window))
old_point = marker_position (XWINDOW (data->current_window)->pointm);
else
old_point = PT;
@@ -5498,7 +5490,7 @@
window's cursor from being copied from another window. */
if (EQ (XWINDOW (data->current_window)->contents, new_current_buffer)
/* If current_window = selected_window, its point is in BUF_PT. */
- && !EQ (selected_window, data->current_window))
+ && !EQ (Fselected_window (), data->current_window))
old_point = marker_position (XWINDOW (data->current_window)->pointm);
else
old_point = BUF_PT (XBUFFER (new_current_buffer));
@@ -5578,9 +5570,9 @@
window holds garbage.) We do this now, before
restoring the window contents, and prevent it from
being done later on when we select a new window. */
- if (! NILP (XWINDOW (selected_window)->contents))
+ if (! NILP (SELECTED_WINDOW ()->contents))
{
- w = XWINDOW (selected_window);
+ w = SELECTED_WINDOW ();
set_marker_both (w->pointm,
w->contents,
BUF_PT (XBUFFER (w->contents)),
@@ -5798,9 +5790,7 @@
in an inconsistent state (e.g. before frame-focus redirection is
canceled). */
select_window (data->current_window, Qnil, 1);
- BVAR (XBUFFER (XWINDOW (selected_window)->contents),
- last_selected_window)
- = selected_window;
+ BVAR (SELECTED_BUFFER (), last_selected_window) = Fselected_window ();
/* Fselect_window will have made f the selected frame, so we
reselect the proper frame here. Fhandle_switch_frame will change the
@@ -6025,7 +6015,7 @@
/* Save w's value of point in the window configuration. If w
is the selected window, then get the value of point from
the buffer; pointm is garbage in the selected window. */
- if (EQ (window, selected_window))
+ if (w == SELECTED_WINDOW ())
p->pointm = build_marker (XBUFFER (w->contents),
BUF_PT (XBUFFER (w->contents)),
BUF_PT_BYTE (XBUFFER (w->contents)));
@@ -6541,7 +6531,6 @@
XSETFRAME (selected_frame, f);
Vterminal_frame = selected_frame;
minibuf_window = f->minibuffer_window;
- selected_window = f->selected_window;
window_initialized = 1;
}
=== modified file 'src/window.h'
--- src/window.h 2013-11-29 05:25:25 +0000
+++ src/window.h 2013-11-29 11:16:03 +0000
@@ -846,12 +846,19 @@
|| WINDOW_RIGHT_MARGIN_COLS (w) == 0)))
/* This is the window in which the terminal's cursor should be left when
- nothing is being done with it. This must always be a leaf window, and its
- buffer is selected by the top level editing loop at the end of each command.
-
- This value is always the same as FRAME_SELECTED_WINDOW (selected_frame). */
-
-extern Lisp_Object selected_window;
+ nothing is being done with it. This must always be a leaf window, and
+ its buffer is selected by the top level editing loop at the end of each
+ command. At the very early initialization, this may be NULL. */
+
+#define SELECTED_WINDOW() \
+ ((FRAMEP (selected_frame) && FRAME_LIVE_P (XFRAME (selected_frame))) \
+ ? XWINDOW (FRAME_SELECTED_WINDOW (XFRAME (selected_frame))) : NULL)
+
+/* This is the buffer displayed by the window above.
+ Note that this is not the same as current_buffer. */
+
+#define SELECTED_BUFFER() \
+ (eassert (SELECTED_WINDOW ()), XBUFFER (SELECTED_WINDOW ()->contents))
/* This is a time stamp for window selection, so we can find the least
recently used window. Its only users are Fselect_window,
=== modified file 'src/xdisp.c'
--- src/xdisp.c 2013-11-29 08:53:50 +0000
+++ src/xdisp.c 2013-11-29 11:20:47 +0000
@@ -638,7 +638,7 @@
if (count > 0)
{
/* ... it's visible in other window than selected, */
- if (count > 1 || b != XBUFFER (XWINDOW (selected_window)->contents))
+ if (count > 1 || b != SELECTED_BUFFER ())
redisplay_other_windows ();
/* Even if we don't set windows_or_buffers_changed, do set `redisplay'
so that if we later set windows_or_buffers_changed, this buffer will
@@ -1256,7 +1256,7 @@
{
struct it it;
struct text_pos pt;
- struct window *w = XWINDOW (selected_window);
+ struct window *w = SELECTED_WINDOW ();
SET_TEXT_POS (pt, PT, PT_BYTE);
start_display (&it, w, pt);
@@ -10869,7 +10869,7 @@
wset_redisplay (XWINDOW (mini_window));
}
}
- else if (!EQ (mini_window, selected_window))
+ else if (!EQ (mini_window, Fselected_window ()))
wset_redisplay (XWINDOW (mini_window));
/* Last displayed message is now the current message. */
@@ -10880,7 +10880,7 @@
/* Prevent redisplay optimization in redisplay_internal by resetting
this_line_start_pos. This is done because the mini-buffer now
displays the message instead of its buffer text. */
- if (EQ (mini_window, selected_window))
+ if (EQ (mini_window, Fselected_window ()))
CHARPOS (this_line_start_pos) = 0;
return window_height_changed_p;
@@ -10921,9 +10921,9 @@
XSETWINDOW (window, w);
if (MINI_WINDOW_P (w))
return 0;
- else if (EQ (window, selected_window))
+ else if (EQ (window, Fselected_window ()))
return 0;
- else if (MINI_WINDOW_P (XWINDOW (selected_window))
+ else if (MINI_WINDOW_P (SELECTED_WINDOW ())
&& EQ (window, Vminibuf_scroll_window))
/* This special window can't be frozen too. */
return 0;
@@ -11163,7 +11163,7 @@
mode_line_noprop_buf; then display the title. */
record_unwind_protect (unwind_format_mode_line,
format_mode_line_unwind_data
- (f, current_buffer, selected_window, 0));
+ (f, current_buffer, Fselected_window (), 0));
Fselect_window (f->selected_window, Qt);
set_buffer_internal_1
@@ -11481,10 +11481,7 @@
fast_set_selected_frame (Lisp_Object frame)
{
if (!EQ (selected_frame, frame))
- {
- selected_frame = frame;
- selected_window = XFRAME (frame)->selected_window;
- }
+ selected_frame = frame;
}
/* Update the tool-bar item list for frame F. This has to be done
@@ -11546,14 +11543,6 @@
GCPRO1 (new_tool_bar);
- /* We must temporarily set the selected frame to this frame
- before calling tool_bar_items, because the calculation of
- the tool-bar keymap uses the selected frame (see
- `tool-bar-make-keymap' in tool-bar.el). */
- eassert (EQ (selected_window,
- /* Since we only explicitly preserve selected_frame,
- check that selected_window would be redundant. */
- XFRAME (selected_frame)->selected_window));
record_unwind_protect (fast_set_selected_frame, selected_frame);
XSETFRAME (frame, f);
fast_set_selected_frame (frame);
@@ -12483,7 +12472,7 @@
saved_current_buffer = current_buffer;
current_buffer = XBUFFER (w->contents);
- if (w == XWINDOW (selected_window))
+ if (w == SELECTED_WINDOW ())
pt = PT;
else
pt = clip_to_bounds (BEGV, marker_position (w->pointm), ZV);
@@ -12916,7 +12905,7 @@
check. */
if (!b->clip_changed && w->window_end_valid)
{
- ptrdiff_t pt = (w == XWINDOW (selected_window)
+ ptrdiff_t pt = (w == SELECTED_WINDOW ()
? PT : marker_position (w->pointm));
if ((w->current_matrix->buffer != b || pt != w->last_point)
@@ -12963,7 +12952,7 @@
static void
redisplay_internal (void)
{
- struct window *w = XWINDOW (selected_window);
+ struct window *w = SELECTED_WINDOW ();
struct window *sw;
struct frame *fr;
int pending;
@@ -13082,7 +13071,7 @@
/* do_pending_window_change could change the selected_window due to
frame resizing which makes the selected window too small. */
- if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
+ if ((w = SELECTED_WINDOW ()) != sw)
sw = w;
/* Clear frames marked as garbaged. */
@@ -13117,7 +13106,7 @@
&& minibuf_level == 0
/* If the mini-window is currently selected, this means the
echo-area doesn't show through. */
- && !MINI_WINDOW_P (XWINDOW (selected_window))))
+ && !MINI_WINDOW_P (SELECTED_WINDOW ())))
{
int window_height_changed_p = echo_area_display (0);
@@ -13143,7 +13132,7 @@
clear_garbaged_frames ();
}
}
- else if (EQ (selected_window, minibuf_window)
+ else if (EQ (Fselected_window (), minibuf_window)
&& (current_buffer->clip_changed || window_outdated (w))
&& resize_mini_window (w, 0))
{
@@ -13327,8 +13316,7 @@
{
do_pending_window_change (1);
/* If selected_window changed, redisplay again. */
- if (WINDOWP (selected_window)
- && (w = XWINDOW (selected_window)) != sw)
+ if ((w = SELECTED_WINDOW ()) != sw)
goto retry;
/* We used to always goto end_of_redisplay here, but this
@@ -13472,8 +13460,6 @@
}
}
- eassert (EQ (XFRAME (selected_frame)->selected_window, selected_window));
-
if (!pending)
{
/* Do the mark_window_display_accurate after all windows have
@@ -13497,10 +13483,10 @@
Lisp_Object mini_window = FRAME_MINIBUF_WINDOW (sf);
struct frame *mini_frame;
- displayed_buffer = XBUFFER (XWINDOW (selected_window)->contents);
+ displayed_buffer = SELECTED_BUFFER ();
/* Use list_of_error, not Qerror, so that
we catch only errors and don't run the debugger. */
- internal_condition_case_1 (redisplay_window_1, selected_window,
+ internal_condition_case_1 (redisplay_window_1, Fselected_window (),
list_of_error,
redisplay_window_error);
if (update_miniwindow_p)
@@ -13524,10 +13510,10 @@
if (FRAME_VISIBLE_P (sf) && !FRAME_OBSCURED_P (sf))
{
- if (hscroll_windows (selected_window))
+ if (hscroll_windows (Fselected_window ()))
goto retry;
- XWINDOW (selected_window)->must_be_updated_p = 1;
+ SELECTED_WINDOW ()->must_be_updated_p = 1;
pending = update_frame (sf, 0, 0);
sf->cursor_type_changed = 0;
}
@@ -13626,7 +13612,7 @@
/* If we just did a pending size change, or have additional
visible frames, or selected_window changed, redisplay again. */
if ((windows_or_buffers_changed && !pending)
- || (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw))
+ || ((w = SELECTED_WINDOW ()) != sw))
goto retry;
/* Clear the face and image caches.
@@ -13731,7 +13717,7 @@
w->last_cursor_vpos = w->cursor.vpos;
w->last_cursor_off_p = w->cursor_off_p;
- if (w == XWINDOW (selected_window))
+ if (w == SELECTED_WINDOW ())
w->last_point = BUF_PT (b);
else
w->last_point = marker_position (w->pointm);
@@ -14461,7 +14447,7 @@
w->cursor.vpos = MATRIX_ROW_VPOS (row, matrix) + dvpos;
w->cursor.y = row->y + dy;
- if (w == XWINDOW (selected_window))
+ if (w == SELECTED_WINDOW ())
{
if (!row->continued_p
&& !MATRIX_ROW_CONTINUATION_LINE_P (row)
@@ -15530,7 +15516,7 @@
/* Point refers normally to the selected window. For any other
window, set up appropriate value. */
- if (!EQ (window, selected_window))
+ if (!EQ (window, Fselected_window ()))
{
ptrdiff_t new_pt = marker_position (w->pointm);
ptrdiff_t new_pt_byte = marker_byte_position (w->pointm);
@@ -15705,7 +15691,7 @@
TEMP_SET_PT_BOTH (MATRIX_ROW_START_CHARPOS (row),
MATRIX_ROW_START_BYTEPOS (row));
- if (w != XWINDOW (selected_window))
+ if (w != SELECTED_WINDOW ())
set_marker_both (w->pointm, Qnil, PT, PT_BYTE);
else if (current_buffer == old)
SET_TEXT_POS (lpoint, PT, PT_BYTE);
@@ -18084,7 +18070,7 @@
glyphs in short form, otherwise show glyphs in long form. */)
(Lisp_Object glyphs)
{
- struct window *w = XWINDOW (selected_window);
+ struct window *w = SELECTED_WINDOW ();
struct buffer *buffer = XBUFFER (w->contents);
fprintf (stderr, "PT = %"pI"d, BEGV = %"pI"d. ZV = %"pI"d\n",
@@ -18115,11 +18101,10 @@
GLYPH > 1 or omitted means dump glyphs in long form. */)
(Lisp_Object row, Lisp_Object glyphs)
{
- struct glyph_matrix *matrix;
+ struct glyph_matrix *matrix = SELECTED_WINDOW ()->current_matrix;
EMACS_INT vpos;
CHECK_NUMBER (row);
- matrix = XWINDOW (selected_window)->current_matrix;
vpos = XINT (row);
if (vpos >= 0 && vpos < matrix->nrows)
dump_glyph_row (MATRIX_ROW (matrix, vpos),
@@ -20114,7 +20099,7 @@
Value is the new character position of point. */)
(Lisp_Object direction)
{
- struct window *w = XWINDOW (selected_window);
+ struct window *w = SELECTED_WINDOW ();
struct buffer *b = XBUFFER (w->contents);
struct glyph_row *row;
int dir;
@@ -20763,7 +20748,7 @@
/* Point refers normally to the selected window. For any
other window, set up appropriate value. */
- if (!EQ (window, selected_window))
+ if (!EQ (window, Fselected_window ()))
{
struct text_pos pt;
@@ -20797,7 +20782,7 @@
static int
display_mode_lines (struct window *w)
{
- Lisp_Object old_selected_window = selected_window;
+ struct window *osw = SELECTED_WINDOW ();
Lisp_Object old_selected_frame = selected_frame;
Lisp_Object new_frame = w->frame;
Lisp_Object old_frame_selected_window = XFRAME (new_frame)->selected_window;
@@ -20806,8 +20791,7 @@
selected_frame = new_frame;
/* FIXME: If we were to allow the mode-line's computation changing the buffer
or window's point, then we'd need select_window_1 here as well. */
- XSETWINDOW (selected_window, w);
- XFRAME (new_frame)->selected_window = selected_window;
+ fset_selected_window (XFRAME (new_frame), make_lisp_ptr (w, Lisp_Vectorlike));
/* These will be set while the mode line specs are processed. */
line_number_displayed = 0;
@@ -20815,10 +20799,8 @@
if (WINDOW_WANTS_MODELINE_P (w))
{
- struct window *sel_w = XWINDOW (old_selected_window);
-
/* Select mode line face based on the real selected window. */
- display_mode_line (w, CURRENT_MODE_LINE_FACE_ID_3 (sel_w, sel_w, w),
+ display_mode_line (w, CURRENT_MODE_LINE_FACE_ID_3 (osw, osw, w),
BVAR (current_buffer, mode_line_format));
++n;
}
@@ -20830,9 +20812,8 @@
++n;
}
- XFRAME (new_frame)->selected_window = old_frame_selected_window;
+ fset_selected_window (XFRAME (new_frame), old_frame_selected_window);
selected_frame = old_selected_frame;
- selected_window = old_selected_window;
return n;
}
@@ -21548,7 +21529,7 @@
face = Qnil;
face_id = (NILP (face) || EQ (face, Qdefault)) ? DEFAULT_FACE_ID
- : EQ (face, Qt) ? (EQ (window, selected_window)
+ : EQ (face, Qt) ? (EQ (window, Fselected_window ())
? MODE_LINE_FACE_ID : MODE_LINE_INACTIVE_FACE_ID)
: EQ (face, Qmode_line) ? MODE_LINE_FACE_ID
: EQ (face, Qmode_line_inactive) ? MODE_LINE_INACTIVE_FACE_ID
@@ -21563,7 +21544,7 @@
record_unwind_protect (unwind_format_mode_line,
format_mode_line_unwind_data
(XFRAME (WINDOW_FRAME (w)),
- old_buffer, selected_window, 1));
+ old_buffer, Fselected_window (), 1));
mode_line_proptrans_alist = Qnil;
Fselect_window (window, Qt);
=== modified file 'src/xterm.c'
--- src/xterm.c 2013-11-29 01:22:40 +0000
+++ src/xterm.c 2013-11-29 11:02:47 +0000
@@ -6665,13 +6665,13 @@
will be selected only when it is active. */
if (WINDOWP (window)
&& !EQ (window, last_mouse_window)
- && !EQ (window, selected_window)
+ && !EQ (window, Fselected_window ())
/* For click-to-focus window managers
create event iff we don't leave the
selected frame. */
&& (focus_follows_mouse
|| (EQ (XWINDOW (window)->frame,
- XWINDOW (selected_window)->frame))))
+ SELECTED_WINDOW ()->frame))))
{
inev.ie.kind = SELECT_WINDOW_EVENT;
inev.ie.frame_or_window = window;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Getting rid of selected_window
2013-11-29 11:53 [RFC] Getting rid of selected_window Dmitry Antipov
@ 2013-11-29 14:38 ` Stefan Monnier
2013-11-29 15:19 ` Eli Zaretskii
1 sibling, 0 replies; 7+ messages in thread
From: Stefan Monnier @ 2013-11-29 14:38 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: Emacs development discussions
> I'm trying to get rid of selected_window, with an intent to completely
> cleanup corner cases where FRAME_SELECTED_WINDOW (selected_frame) is
> not the same as selected_window and avoid redundant synchronization of
> them. Comments are welcome.
Indeed, I think that the two are now 100% synchronized, so we should be
able to get rid of `selected-window'.
A few comments, tho:
- Shouldn't we replace uses of `selected_window' with a call to an
inlined function? I.e. either make Fselected_window inlinable?
- If you want SELECTED_WINDOW (which should be an inlined function),
then I think we should define it as XWINDOW (Fselected_window ())
rather than providing a redundant definition.
- SELECTED_BUFFER should be a function rather than a macro and
it should return a Lisp_Object rather than a struct buffer*.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Getting rid of selected_window
2013-11-29 11:53 [RFC] Getting rid of selected_window Dmitry Antipov
2013-11-29 14:38 ` Stefan Monnier
@ 2013-11-29 15:19 ` Eli Zaretskii
2013-11-29 16:54 ` Stefan Monnier
1 sibling, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2013-11-29 15:19 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: emacs-devel
> Date: Fri, 29 Nov 2013 15:53:46 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
>
> I'm trying to get rid of selected_window, with an intent to completely cleanup corner
> cases where FRAME_SELECTED_WINDOW (selected_frame) is not the same as selected_window
> and avoid redundant synchronization of them.
What for? I don't think you can get rid of the need to synchronize
those, because of features like focus redirection and minibuffer-only
frames.
> Comments are welcome.
Here are some:
> === modified file 'src/cmds.c'
> --- src/cmds.c 2013-09-05 02:27:13 +0000
> +++ src/cmds.c 2013-11-29 11:02:47 +0000
> @@ -282,7 +282,7 @@
> nonundocount = 0;
>
> if (NILP (Vexecuting_kbd_macro)
> - && !EQ (minibuf_window, selected_window))
> + && !EQ (minibuf_window, Fselected_window ()))
> {
> if (nonundocount <= 0 || nonundocount >= 20)
> {
>
> === modified file 'src/dispextern.h'
> --- src/dispextern.h 2013-11-06 00:14:56 +0000
> +++ src/dispextern.h 2013-11-29 11:02:47 +0000
> @@ -1410,7 +1410,7 @@
>
> #define CURRENT_MODE_LINE_FACE_ID_3(SELW, MBW, SCRW) \
> ((!mode_line_in_non_selected_windows \
> - || (SELW) == XWINDOW (selected_window) \
> + || (SELW) == SELECTED_WINDOW () \
I don't understand what is the difference between Fselected_window and
SELECTED_WINDOW. Why do we use both in C? It's confusing.
> --- src/editfns.c 2013-11-29 01:22:40 +0000
> +++ src/editfns.c 2013-11-29 11:02:47 +0000
> @@ -832,8 +832,8 @@
> ? Fcopy_marker (BVAR (current_buffer, mark), Qnil)
> : Qnil),
> /* Selected window if current buffer is shown in it, nil otherwise. */
> - (EQ (XWINDOW (selected_window)->contents, Fcurrent_buffer ())
> - ? selected_window : Qnil),
> + (EQ (SELECTED_WINDOW ()->contents, Fcurrent_buffer ())
> + ? Fselected_window () : Qnil),
SELECTED_WINDOW can return NULL, and then this dereference will
segfault.
> --- src/fileio.c 2013-11-27 16:08:53 +0000
> +++ src/fileio.c 2013-11-29 11:13:44 +0000
> @@ -3868,8 +3868,8 @@
>
> /* If display currently starts at beginning of line,
> keep it that way. */
> - if (XBUFFER (XWINDOW (selected_window)->contents) == current_buffer)
> - XWINDOW (selected_window)->start_at_line_beg = !NILP (Fbolp ());
> + if (SELECTED_BUFFER () == current_buffer)
> + SELECTED_WINDOW ()->start_at_line_beg = !NILP (Fbolp ());
Same here (and elsewhere in the patch).
> +#define SELECTED_WINDOW() \
> + ((FRAMEP (selected_frame) && FRAME_LIVE_P (XFRAME (selected_frame))) \
> + ? XWINDOW (FRAME_SELECTED_WINDOW (XFRAME (selected_frame))) : NULL)
This changes the semantics of selected_window, because it requires
that its frame be live. I don't think we understand the effects of
this change.
> +/* This is the buffer displayed by the window above.
> + Note that this is not the same as current_buffer. */
> +
> +#define SELECTED_BUFFER() \
> + (eassert (SELECTED_WINDOW ()), XBUFFER (SELECTED_WINDOW ()->contents))
So now we have SELECTED_BUFFER and current_buffer, which are
different in unspecified ways. What do you think is the probability
that someone will use the wrong one of that pair? I think it's 100%.
> /* do_pending_window_change could change the selected_window due to
> frame resizing which makes the selected window too small. */
> - if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
> + if ((w = SELECTED_WINDOW ()) != sw)
> sw = w;
What if SELECTED_WINDOW returns NULL? You have now assigned NULL to
sw. The old code did better.
> @@ -13117,7 +13106,7 @@
> && minibuf_level == 0
> /* If the mini-window is currently selected, this means the
> echo-area doesn't show through. */
> - && !MINI_WINDOW_P (XWINDOW (selected_window))))
> + && !MINI_WINDOW_P (SELECTED_WINDOW ())))
MINI_WINDOW_P will segfault if SELECTED_WINDOW returns NULL.
> @@ -18115,11 +18101,10 @@
> GLYPH > 1 or omitted means dump glyphs in long form. */)
> (Lisp_Object row, Lisp_Object glyphs)
> {
> - struct glyph_matrix *matrix;
> + struct glyph_matrix *matrix = SELECTED_WINDOW ()->current_matrix;
> EMACS_INT vpos;
>
> CHECK_NUMBER (row);
> - matrix = XWINDOW (selected_window)->current_matrix;
This will segfault where the old code didn't (if 'row' was not a
number).
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Getting rid of selected_window
2013-11-29 15:19 ` Eli Zaretskii
@ 2013-11-29 16:54 ` Stefan Monnier
2013-11-29 19:42 ` Eli Zaretskii
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2013-11-29 16:54 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Dmitry Antipov, emacs-devel
> What for? I don't think you can get rid of the need to synchronize
> those, because of features like focus redirection and minibuffer-only
> frames.
AFAIK these don't interact at all. IOW they're unrelated.
> I don't understand what is the difference between Fselected_window and
> SELECTED_WINDOW. Why do we use both in C? It's confusing.
Indeed, it is confusing. Part of the confusion comes from the current
code, tho. This is an opportunity to clarify the code.
> SELECTED_WINDOW can return NULL, and then this dereference will
> segfault.
This is the crucial part of the difficulty: figure out which part of the
code needs to pay attention to the case where there's no selected-window yet.
I think SELECTED_WINDOW should never return NULL. Instead, we should
have a new function to test if there is a selected-window, and call it
explicitly before SELECTED_WINDOW, at those rare places where such is needed.
> This changes the semantics of selected_window, because it requires
> that its frame be live.
Indeed the Fframe_live_p check shouldn't be in there. If there are
places where such a check is needed, then let's add the check at those
specific places.
> So now we have SELECTED_BUFFER and current_buffer, which are
> different in unspecified ways.
Yes, we now have it, and this patch won't change that, other than giving
a name to the concept of "selected buffer".
> What do you think is the probability that someone will use the wrong
> one of that pair? I think it's 100%.
That's already a risk and such errors already happen. This said,
I think that instead of selected_buffer, we should use the name
selected_window_buffer, to make things more clear.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Getting rid of selected_window
2013-11-29 16:54 ` Stefan Monnier
@ 2013-11-29 19:42 ` Eli Zaretskii
2013-12-02 14:37 ` Dmitry Antipov
0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2013-11-29 19:42 UTC (permalink / raw)
To: Stefan Monnier; +Cc: dmantipov, emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Dmitry Antipov <dmantipov@yandex.ru>, emacs-devel@gnu.org
> Date: Fri, 29 Nov 2013 11:54:13 -0500
>
> > What for? I don't think you can get rid of the need to synchronize
> > those, because of features like focus redirection and minibuffer-only
> > frames.
>
> AFAIK these don't interact at all. IOW they're unrelated.
Sorry, I'm unconvinced.
> > So now we have SELECTED_BUFFER and current_buffer, which are
> > different in unspecified ways.
>
> Yes, we now have it, and this patch won't change that, other than giving
> a name to the concept of "selected buffer".
But that's exactly the problem: that we now give a catchy name to that
other thing. As long as it is unnamed, the chances of it being used
by uninitiated are slim. But once you name it, it's very easy to
become confused between them, and use the wrong one.
> > What do you think is the probability that someone will use the wrong
> > one of that pair? I think it's 100%.
>
> That's already a risk and such errors already happen.
See above: I don't think so.
> This said, I think that instead of selected_buffer, we should use
> the name selected_window_buffer, to make things more clear.
I suggest buffer_of_selected_window, if we must name this thing. I'd
prefer to leave it without a name.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Getting rid of selected_window
2013-11-29 19:42 ` Eli Zaretskii
@ 2013-12-02 14:37 ` Dmitry Antipov
2013-12-02 15:06 ` Stefan Monnier
0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Antipov @ 2013-12-02 14:37 UTC (permalink / raw)
To: Eli Zaretskii, Stefan Monnier; +Cc: emacs-devel
Thanks for your suggestions.
BTW, there is another approach: what if we introduce Lisp_Fwd_Frame_Obj
and forward Vselected_window to SELECTED_FRAME()'s selected_window?
Dmitry
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Getting rid of selected_window
2013-12-02 14:37 ` Dmitry Antipov
@ 2013-12-02 15:06 ` Stefan Monnier
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Monnier @ 2013-12-02 15:06 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: Eli Zaretskii, emacs-devel
> BTW, there is another approach: what if we introduce Lisp_Fwd_Frame_Obj
> and forward Vselected_window to SELECTED_FRAME()'s selected_window?
Please don't! Frame-local vars are on the way to extinction and it's
a good thing.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-12-02 15:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-29 11:53 [RFC] Getting rid of selected_window Dmitry Antipov
2013-11-29 14:38 ` Stefan Monnier
2013-11-29 15:19 ` Eli Zaretskii
2013-11-29 16:54 ` Stefan Monnier
2013-11-29 19:42 ` Eli Zaretskii
2013-12-02 14:37 ` Dmitry Antipov
2013-12-02 15:06 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).