Guillaume Munch wrote: > I replace stored alignment and spacing values with computed values, only > in the case of specific grids under discussion (Eqnarray, AMS...). Thus, > defaultColAlign() can still make sense in the future for grids that rely > on stored alignment and spacing, and I do not see what makes it useful > to remove it entirely.
OK, I see. My argument for removing defaultColAlign() was that we do not want to have dead code, but I overlooked that it is still used in the base class, so the only thing which could be removed is the "virtual" keyword (but I don't have a strong opinion on that). > About "defaultColAlign() is not only used for display": you mean that it > is risky to change the behaviour of stored values. Let us admit that > there is still a risk despite all my checks. Then, I proposed to keep > the default values to be extra safe just above your remark which makes > me think that you may have missed it. I believe that I understood the intention of your patch. However, I found surprising behaviour in mathed in the past, and the motivation for my comments was to ensure that you do not trap into this pitfall as well. > (But in any case these stored values are necessarily wrong since they > are not saved to the file for the environments under consideration. I am > highly sceptical that a bug could remain open for so long if it caused > more than a display bug to which users got accustomed.) There are some hidden parts of mathed which have no user interface. I would not bet that these settings are not written under any circumstances (but I agree that a bug in the usual cases would have been found earlier, so we can assume they work). > Again this misses the point. There is nothing to "repair" about > defaultColSpace(). If there is still any buggy behaviour regarding the > spacing or alignment then they should be changed to read the computed > values that I introduce. Same remark about keeping defaultColSpace in > order to be extra safe. I am sorry, you are right, I was not careful anymore at the end of the message. > If anything, please continue! Also, this is a software that many people > (including me) depend on professionally, so I do not understand this > notion that developing LyX is supposed to be "Spaß". Well, in total it is supposed to make fun (at least for me, otherwise I would stop contributing). > See the attached for two "safe" (and easy to read) patches, if you agree > that a safe patch that we have been discussing for a month can still get > in for 2.2. I started to review the patch, but stopped when I found a difference between the old and new behaviour of InsetMathHull::defaultColAlign() for hullRegexp, since I don't have so much time at the moment. Again, this combination is most likely not used, but "most likely" is not enough IMHO for a beta. Why is it so important to fix this bug before 2.2.0 when it is known for so many years already? BTW, it did annoy me a lot when I wrote my thesis (therefore my attempt to fix it many vears ago), but since I did not find a proper solution at that time I simply got used to the wrong display (which was not too difficult). If you were proposing to fix the bug at the beginning of the 2.3 development cycle then we could skip most parts of this discussion and you could simply submit it. As I understand it, we do all agree that the proposed change is good, the only differences are in judging the risk. Georg
