Re: Preview: portable dumper

[Eli Zaretskii]

> Date: Sun, 4 Dec 2016 12:20:33 +0000
> Cc: address@hidden, address@hidden, address@hidden
> From: Alan Mackenzie <address@hidden>
> 
> 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 hope it's the latter.

> > > (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.

I've re-read those threads and found my participation in them to be
almost non-existent.  In the first one it was limited to presenting
benchmarks of the performance that was discussed.  In the second one I
barely said anything at all, and when I did, it was to encourage you
to install your patch on the release branch rather than master.

> >   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.

Once again, a patch that was discussed at length and tested by several
people was committed that fixed the problem.  I can understand your
frustration, but we really cannot install more than a single patch for
each problem.

> > > (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.

Yes, because those touched very fundamental functionalities in
manipulating buffer text, which I didn't want to destabilize without
having a test suite with good coverage.

So in sum, I feel that these incidents don't have a lot in common with
the issues discussed here.

> > > 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.

I think treating code in C as more delicate is prudent: changes there
will almost always have wider implications than changes in Lisp, and
the code is frequently harder to understand, so better testing is IMO
justified.