[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Preview: portable dumper
From: |
Eli Zaretskii |
Subject: |
Re: Preview: portable dumper |
Date: |
Sun, 04 Dec 2016 17:53:43 +0200 |
> 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.
- Re: Preview: portable dumper, (continued)
- Re: Preview: portable dumper, Philippe Vaucher, 2016/12/15
- Re: Preview: portable dumper, Eli Zaretskii, 2016/12/02
- Re: Preview: portable dumper, Daniel Colascione, 2016/12/02
- Re: Preview: portable dumper, Eli Zaretskii, 2016/12/03
- Re: Preview: portable dumper, Daniel Colascione, 2016/12/03
- Re: Preview: portable dumper, Eli Zaretskii, 2016/12/03
- Re: Preview: portable dumper, Alan Mackenzie, 2016/12/03
- Re: Preview: portable dumper, Eli Zaretskii, 2016/12/03
- Re: Preview: portable dumper, Alan Mackenzie, 2016/12/04
- Re: Preview: portable dumper, Dmitry Gutov, 2016/12/04
- Re: Preview: portable dumper,
Eli Zaretskii <=
- Re: Preview: portable dumper, Daniel Colascione, 2016/12/03
- Re: Preview: portable dumper, Dmitry Gutov, 2016/12/03
- Re: Preview: portable dumper, Stefan Monnier, 2016/12/03
- Re: Preview: portable dumper, Daniel Colascione, 2016/12/03
- Re: Preview: portable dumper, Stefan Monnier, 2016/12/03
- Re: Preview: portable dumper, Alan Mackenzie, 2016/12/04
- Re: Preview: portable dumper, Dmitry Gutov, 2016/12/04
- Re: Preview: portable dumper, Stefan Monnier, 2016/12/04
- Re: Preview: portable dumper, Alan Mackenzie, 2016/12/04
- forward-comment and syntax-ppss (was: Preview: portable dumper), Stefan Monnier, 2016/12/04