bug#5718: scroll-margin in buffer with small line count.

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 5718@debbugs.gnu.org,  ahyatt@gmail.com,  gavenkoa@gmail.com
> Date: Sun, 22 Jan 2017 12:21:20 -0500
> 
> > When you copy the iterator structure and modify the copy, you need to
> > save and restore the bidi cache, using SAVE_IT and RESTORE_IT macros.
> > Otherwise, the code which calls this function will work incorrectly if
> > it uses the original iterator after the call, because the bidi cache
> > is not restored to its state before the call.
> 
> I came up with this modified version of partial_line_height, which does
> actually work.

It looks good to me, thanks.

> Having to do RESTORE_IT (&it, &it, it_data) is a bit
> unintuitive though.  I don't understand what the typical use case of
> this macro is, if it's going to copy back the new state into the
> original `it', then why use a separate `it' in the first place?

If you don't modify the original iterator object, then you indeed
don't need to copy back, you only need to restore the bidi cache
state, by calling bidi_unshelve_cache directly, as some code fragments
actually do.  (The macro refrains from copying if the two arguments
point to the same object, so it transparently does this for you.)

More generally, two separate arguments are needed because some of the
macro uses choose one of several copies to copy back.  For example:

  if (result == MOVE_LINE_CONTINUED
      && it->line_wrap == WORD_WRAP
      && wrap_it.sp >= 0
      && ((atpos_it.sp >= 0 && wrap_it.current_x < atpos_it.current_x)
          || (atx_it.sp >= 0 && wrap_it.current_x < atx_it.current_x)))
    RESTORE_IT (it, &wrap_it, wrap_data);
  else if (atpos_it.sp >= 0)
    RESTORE_IT (it, &atpos_it, atpos_data);
  else if (atx_it.sp >= 0)
    RESTORE_IT (it, &atx_it, atx_data);

The general paradigm is:

  . save 'it' in some local variable
  . modify 'it'
  . restore 'it' from the local variable
  . continue using 'it' as if the modification never happened

> By the way, while testing I noticed that `set-window-text-height'
> doesn't take `line-spacing' into account, should it?

No, because it returns its value in canonical line units, which means
no line-spacing.