Re: Preview: portable dumper

[Alan Mackenzie]

Hello, Eli.

On Sat, Dec 03, 2016 at 05:11:33PM +0200, Eli Zaretskii wrote:
> > Date: Sat, 3 Dec 2016 14:36:04 +0000
> > From: Alan Mackenzie <address@hidden>
> > Cc: address@hidden, Daniel Colascione <address@hidden>,
> >     address@hidden

> > However, it feels that an unusually high proportion of C level changes I
> > have hacked or proposed have been rejected.  ("Unusual" when compared to
> > lisp level changes.)

> Can you estimate that proportion?  You cited 3 examples, and then said
> that "a lot of changes have been accepted".  3 out of many is not a
> high proportion.

I can't really estimate it accurately (or it would take far too much
time to do so).  It could be as high as 1 in 3.  It could be as low as 1
in 10.

> > (i) Changing the method of syntax.c scanning backwards over comments.  My
> > changes found their way into branch comment-cache in 2016-03.  Despite
> > this change having been extensively discussed in emacs-devel, and sort of
> > "approved", the final patch was never considered on its merits.  The
> > ostensible reason was that it used a cache which wasn't the syntax-ppss
> > cache.

> I don't think I participated in that discussion or reviewed that
> patch, so I cannot say anything about that.

I've found that thread, and I misremembered it.  The preceding
discussion was not extensive.  It was in the thread with Subject:
"bug#22884: 25.0.92; C/l mode editing takes waaaayy too long" and
essentially consisted of just three emails:
  (i) Alan -> Eli: Thu, 3 Mar 2016 23:18:23 +0000
  (ii) Eli -> Alan: Fri, 04 Mar 2016 10:32:56 +0200
  (iii) Alan -> Eli: Fri, 4 Mar 2016 09:37:07 +0000
.  In (i), I proposed using text properties to cache the string/comment
state of positions, and asked you whether that might overwhelm the text
property mechanism.  In (ii) you replied that it shouldn't.  In (iii) I
announced my intention to hack it, which I then did.

I don't remember you reviewing the patch, or expressing any further
views on it afterwards, so we agree on that.  The discussion after I
submitted my patch happened in the threads Subject: "Re: [Emacs-diffs]
comment-cache 223d16f 2/3: Apply `comment-depth' text properties when
calling `back_comment'." and Subject: "Permission requested to merge
branch comment-cache into master.".  This discussion was long and (to
me) unexpectedly aggressive, and didn't concern itself with the
correctness of the patch.  It seemed intended to prevent the adoption of
my patch by spreading FUD.  If this was indeed the intention, it was
entirely successful.

> > (ii) Around 2015-11-17, I proposed a patch to fix bug #21869 and bug
> > #21333, with top line of the commit message being "Invoke
> > window-size-change-functions after changing echo area height.".  The
> > problem here was that window-size-change-functions was sometimes being
> > called twice.  You rejected my patch because you were "not keen" on
> > changing the order of calls in the display engine because we "didn't
> > fully understand what was going on".  Again, I don't think this proposed
> > patch was really considered on its merits.

> Please see

>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21869#23

> It indicates that we already had a patch for those problems, which was
> already tested quite a lot by that time.  I think it's only natural to
> prefer a well tested and discussed patch to a new one trying to fix
> the same problem.  Reading this message further down:

>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21869#37

> I see that you agreed that the problem was fixed by the patch we had,
> which allowed me to close the bug.

I've read through that debbugs bug archive.  It was a long time ago.  I
remember feeling discouraged that my patch was not considered.  I think
I felt that my patch fixed the fundamental problem in xdisp.c, whereas
the patch actually applied was more of a workaround.  Or something like
that.  But I didn't push the point at the time.

> > (iii) Earlier this year, we were having problems because some primitives
> > were not calling before-change-functions and after-change-functions the
> > way we might wish.  My offer to analyse the code and amend it so that all
> > primitives would call b-c-f and a-c-f predictably was declined, the
> > proviso being (if I remember correctly) "unless somebody writes a solid
> > suite of unit tests".  At the time of this rejection, I'd already
> > invested some time on the analysis.

> That's not exactly my recollection.  The analysis was not rejected, I
> simply already did it when you proposed that, so it wasn't necessary.

OK.  But the notion of acutally fixing the primitives so that they would
each call b-c-f and a-c-f was strongly discouraged, if I remember
correctly.

> > In short, I feel discouraged from working at the C level because of the
> > above.

> Please don't be discouraged.  There's no policy of preventing changes
> on the C level.  However, for some changes which affect important
> functions, I think it's prudent to require a reasonable coverage by
> tests, so that we know we didn't break anything.

Perhaps it is that last point (that C functions are more likely to be
critical to Emacs's working than lisp functions, and therefore the
testing is more important) that is the thing.

-- 
Alan Mackenzie (Nuremberg, Germany).