Re: [patch] Fix bug 8874 Chunk insets behave differently form ERT insets when applying formatting
Jean-Marc Lasgouttes wrote: > Thanks for looking at this. Thanks for the comments, I took all of them into account abnd submitted the patch. Georg
Re: [patch] Fix bug 8874 Chunk insets behave differently form ERT insets when applying formatting
09/02/2014 15:30, Georg Baum:
You mean resetFontEdit() should return return true if isPassThru() returns
true? This would certainly make sense, but I am not sure if we should do it
now. My proposed patch is conservative in the sense that the behaviour is
not changed for any inset except the chunk inset.
OK, apply your path as it is now, it is the safest bet at this stage. I
have added your comment to the bug report for posterity's sake.
Just code comments: can resetFontEdit be rewritten as follows? It seems
clearer to me.
+bool Inset::resetFontEdit() const
+{
+ return getLayout().resetsFont() || !inheritFont();
+}
Also the comment over resetsFontEdit may need a clean up, e.g. the
following snippet:
For copy/paste the operations the language is never changed, since
Thanks for looking at this.
JMarc
Re: [patch] Fix bug 8874 Chunk insets behave differently form ERT insets when applying formatting
Jean-Marc Lasgouttes wrote: > I have a couple questions still. > > * the description I see of ResetsFont in the Customization manual > strikes me as being like the opposite of Inset::inheritFont. How could > an inset have non-compatible values for these two values? Would it make > any sense? Or is one of the value for screen display and the other for > LaTeX output? In this case do we need a InheritFont InsetLayout parameter? The method name is more clear than the layout tag¹: Inset::resetFontEdit() defines the behaviour of the inset for editing operations like changing the language or font size. Inset::inheritFont() describes how the inset is treated by LaTeX, and therefore whether LyX needs to export outside font changes in the inset again or not. You are right, in most cases these two methods are just the negation of each other. The only exceptions are currently Flex insets (except the URL inset): Both inheritFont() and resetFontEdit() return true. After revisiting my changes at http://www.lyx.org/trac/changeset/f176a184/lyxgit I currently do not see anymore why the separation was needed: The same effect could have been achieved by making inheritFont() customizable in the layout files. I am also not sure whether the conflicting values of resetFontEdit() and inheritFont() for the flex insets do make sense. I suggest to revisit these after applying the patch, since after the layout2layout conversion it will be explicit which insets need to be looked at. For the future we could also look at merging resetFontEdit() and inheritFont() again, unless somebody finds a reason not to do so (or I remember again the real cause for the separation). For now I think this would be too dangerous. > * concerning resetFontEdit, one might think that using isPassThru would > make sense somewhere. You mean resetFontEdit() should return return true if isPassThru() returns true? This would certainly make sense, but I am not sure if we should do it now. My proposed patch is conservative in the sense that the behaviour is not changed for any inset except the chunk inset. Georg ¹: I now believe that I was a bit on the wrong track when I used the ResetsFont layout tag (which Richard initially introduced in order to fix bug 5657) for the editing purposes, however, it has been like that for several years now.
Re: [patch] Fix bug 8874 Chunk insets behave differently form ERT insets when applying formatting
06/02/2014 22:14, Georg Baum: This is one of three bugs scheduled for 2.1.0: If you apply font changes, e.g. small size to a paragraph including a chunk inset, the result will not be compilable anymore (only after saving and reloading the document). Jean-Marc did the detective work and found out the reason: resetFontEdit() returns the wrong value for chunk insets. The attached patch fixes that by setting the correct value in the layout file, and making the defaults of resetFontEdit() and inheritFont() compatible. See http://www.lyx.org/trac/ticket/8874 if you want to understand all details. I tested this patch extensively, and it works fine now (it is the third version), but since it involves a layout file format change I'd rather ask: Is it OK to go in? Of course I would convert all other layout files at the same time. I have a couple questions still. * the description I see of ResetsFont in the Customization manual strikes me as being like the opposite of Inset::inheritFont. How could an inset have non-compatible values for these two values? Would it make any sense? Or is one of the value for screen display and the other for LaTeX output? In this case do we need a InheritFont InsetLayout parameter? * concerning resetFontEdit, one might think that using isPassThru would make sense somewhere. JMarc
Re: [patch] Fix bug 8874 Chunk insets behave differently form ERT insets when applying formatting
Georg Baum wrote: > I tested this patch extensively, and it works fine now (it is the third > version), but since it involves a layout file format change I'd rather ask: > Is it OK to go in? I am all for it. Regards, Jürgen
