On 10/24/2017 01:29 AM, Scott Kostyshak wrote: > On Mon, Oct 23, 2017 at 08:44:07AM +0000, Enrico Forestieri wrote: >> On Sun, Oct 22, 2017 at 09:36:56PM -0400, Richard Heck wrote: >>> On 10/22/2017 06:19 PM, Scott Kostyshak wrote: >>>> On Sat, Oct 14, 2017 at 06:37:06AM +0000, Jürgen Spitzmüller wrote: >>>> >>>>> Also, I think we should consider Günter's lyx2lyx patch [1], but I >>>>> didn't have time to thoroughly review it myself. >>>> Will anyone have time to take a look at the patch by Wednesday? >>> I could look at the code from a code-triaging point of view, but I have >>> not followed >>> the controversy about this, so someone would need to tell me what the >>> code is supposed >>> to do. >> I don't think this is a change that should be performed at a RC1 stage, >> frankly. It is too risky and of dubious utility. > Thanks for bringing up this concern. The patch is not trivial. Further, > the patch only deals with backwards compatibility. From what I recall, > we place a higher importance on forward conversion, and although we do > make efforts to provide good backward compatibility, I believe that we > have at times knowingly not implemented certain functionality in our > backwards conversion. Indeed, our "Development" manual covers this: > > While the conversion routine is required to produce a document that > is equivalent to the old version, the requirements of the reversion > are not that strict. If possible, try to produce a proper reversion, > using ERT if needed, but for some features this might be too > complicated. In this case, the minimum requirement of the reversion > routine is that it produces a valid document which can be read by an > older LyX. If absolutely needed, even data loss is allowed for the > reversion. > > The current code (without the patch) clearly already satisfies the > "minimum requirement". All this to say: I don't think the patch is too > important for 2.3.0 and in my opinion I'm fine if we do not put it in. > > Since I don't think the patch is critical, and since we are hopefully a > couple of days from going forward with a freeze for RC1, I propose that > we should only consider this patch if a developer gives a "very > confident" +1, and if we have an extensive test suite.
The whole lyx2lyx test suite issue is one we should sort out, but we don't have any such thing at this point. I don't have a strong opinion on the issues that led to this patch, but it seems to me that we aredoing a lot here, too much for this late in the game. The actual problem is the deletion of ZWSP characters, which 72a488d7 said was needed "so that they don't accumulate". The concern here, I take it, is that reverting to a pre-2.3.x format and then re-importing might lead to a stack of these things. Why not just make sure that doesn't happen? I.e., check if there's a stack of them, and if so fix that? Richard
