Le 11/06/2017 à 01:08, Enrico Forestieri a écrit :
On Mon, Oct 17, 2016 at 00:16:21AM +0200, Guillaume Munch wrote:

commit 676a0639c505d52336e3228c44c2515ccbbaf34a
Author: Guillaume Munch <g...@lyx.org>
Date:   Sat Sep 24 00:49:00 2016 +0200

     Use otexstringstream for the captions of InsetCaptionables
* Enable TexRow for InsetListings caption. * Move getCaption* from InsetText to InsetCaptionable. * Clean-up caption generation for InsetFloat.
[...]
@@ -420,7 +417,11 @@ docstring InsetListings::getCaption(OutputParams const & 
runparams) const
        // NOTE that } is not allowed in blah2.
        regex const reg("(.*)\\\\label\\{(.*?)\\}(.*)");
        string const new_cap("$1$3},label={$2");
-       return from_utf8(regex_replace(to_utf8(cap), reg, new_cap));
+       // TexString validity: the substitution preserves the number of 
newlines.
+       // Moreover we assume that $2 does not contain newlines, so that the 
texrow
+       // information remains accurate.
+       cap.str = from_utf8(regex_replace(to_utf8(cap.str), reg, new_cap));
+       return cap;
  }

This commit broke the caption handling in InsetListings. For example,
the document attached at #9382:
https://www.lyx.org/trac/raw-attachment/ticket/9382/problem.20_nopreamble.lyx
now fails to compile. It can be compiled again with the attached patch,
which is not the right one, of course.



Dear Enrico,


I had a look at this issue 8 days ago since you pointed at my commit. I
realised then that my commit did nothing to worsen or improve this
issue (the commit is meant to leave the output unchanged, and the bug
appears in LyX 2.2.2 where this line of commits has never been). Sorry
for the wait: I did not have much time on my hands and it was not a
regression for me so I decided to postpone my reply until I had more time.

I now see that you are proposing a patch at
<http://www.lyx.org/trac/ticket/10705>. I have several questions about
your patch.

Given that you had located a regression (you even point to a specific
part of the code), have you tried to see what was causing the regression
in the commit while coming up with a patch? I think that trying to do so
would show you that the commit you are pointing to actually leaves the
output unchanged.

In your bug report you notice that the bug is present in stable and you
say to Scott that the patch is valid for stable. It does not apply. Why
did you let Scott think that you have tested your patch on stable? I
think that if you had, then you could have noticed that the commit was
never backported.

As you know, you are writing to a developers list where people are
willing to put in their valuable time to help you, and expect accurate
technical informations. It happens to make a mistake, but it does not
make sense to me that with enough care you did not find an occasion to
correct yourself.

I want to be able to trust technical claims that are made on the list in
the future, and so do other developers probably. I try to always
double-check technical claims I make and sometimes it helps me catch
errors at the last moment. I would like to be sure that we agree on
everybody putting in the same care in their contributions.


Guillaume

Reply via email to