Re: [RFC]: replace-region-contents

[Eli Zaretskii]

> From: Tassilo Horn <address@hidden>
> Cc: Paul Eggert <address@hidden>,  address@hidden,  address@hidden
> Date: Fri, 08 Feb 2019 17:23:29 +0100
> 
> if nobody complains, I'd like to install the following patch.

No complaints from me, just a few comments.

> I've set too_expensive to 64 because then my benchmark was almost as
> fast as the original version without `replace-buffer-contents', and I
> couldn't find any difference in observable behavior in that value anyway
> except lower values give much better performance but a too low value
> like 10 will segfault.  My main concern was that point was at the same
> position before and after pretty-printing my JSON.

I'd prefer to expose this to Lisp, not hardcode the value.  We could
use the hardcoded value in json.el, but I'd like to leave this to the
application in the other cases.  I'm not sure such a small value will
always be the right tradeoff, so I think we shouldn't decide just yet.

If you agree with that, let's expose this via a new optional argument
to replace-buffer-contents, and let's treat the value of nil as a very
large integer value, i.e. "no limit".

> --- a/src/editfns.c
> +++ b/src/editfns.c
> @@ -1910,6 +1910,11 @@ determines whether case is significant or ignored.  */)
>  
>  #undef ELEMENT
>  #undef EQUAL
> +#define USE_HEURISTIC
> +
> +#ifdef USE_HEURISTIC
> +#define DIFFSEQ_HEURISTIC
> +#endif
>  
>  /* Counter used to rarely_quit in replace-buffer-contents.  */
>  static unsigned short rbc_quitcounter;
> @@ -2017,8 +2022,11 @@ differences between the two buffers.  */)
>      .insertions = SAFE_ALLOCA (ins_bytes),
>      .fdiag = buffer + size_b + 1,
>      .bdiag = buffer + diags + size_b + 1,
> +#ifdef DIFFSEQ_HEURISTIC
> +    .heuristic = true,
> +#endif

Why do we need this triple-level conditional?  If we think the
heuristic is a good idea, let's just enable it unconditionally.  If
someone wants to modify Emacs on the C level, they can edit the source
as easily as they can invoke the compiler with a -U switch.

Finally, I think this needs NEWS entries, both regarding the new
function and the additional argument to replace-buffer-contents.  The
latter is also documented in the ELisp manual, which will need to be
updated.

Thanks.

P.S.  I still hope Paul will chime in and comments on the diffseq bits
we are modifying here.