On Mon, Apr 25, 2016 at 02:25:29PM -0400, Richard Heck wrote: > On 04/25/2016 01:01 PM, Scott Kostyshak wrote: > > On Mon, Apr 25, 2016 at 11:48:00AM -0400, Richard Heck wrote: > >> On 04/24/2016 10:21 PM, Cyrille Artho wrote: > >>> Scott Kostyshak wrote: > >>>> In 2.1.x, because of a bug in our validator code, we allowed negative > >>>> values to be input into some document settings. This bug has been fixed > >>>> for 2.2.0dev but what that means is that if you open such a document > >>>> with LyX 2.2.0dev, you will not be able to save any changes to document > >>>> settings unless you correct the negative values and it is not obvious to > >>>> the user that this must be done. I can imagine this causing some > >>>> confusion. > >>>> > >>>> For more details, see: > >>>> http://www.lyx.org/trac/ticket/10095#comment:2 > >>>> > >>>> I'm not sure that we want to do anything for 2.2.0, but I wanted to > >>>> check with the list to see the thoughts of others. For example, we could > >>>> change the validators in page margins to allow negative values, which > >>>> seem to work as intended with LaTeX. Or we could provide a warning when > >>>> creating the document settings dialog that explains why the "OK" button > >>>> is disabled. > >>>> > >>>> Scott > >>>> > >>> I would unconditionally allow negative values. They are often used to > >>> fix issues with padding that is too large or incorrect bounding boxes > >>> in images. > >>> > >>> If you suddenly break documents where users relied on negative > >>> margins/spacing to fix other problems, you create more problems than > >>> you solve. > >> So it seems to me that we need to correct the validator. Otherwise, we > >> will break documents, since people can't change the document settings > >> without "fixing" the negative values. > > Seems reasonable to me and probably a safe change. It would be good to > > figure out exactly which settings could suffer from this issue. > > These ones > > ../frontends/qt4/ [master] > grep -P 'unsigned\w*Validator' GuiDocument.cpp > textLayoutModule->indentLE->setValidator(unsignedLengthValidator( > textLayoutModule->skipLE->setValidator(unsignedGlueLengthValidator( > > pageLayoutModule->paperheightLE->setValidator(unsignedLengthValidator( > > pageLayoutModule->paperwidthLE->setValidator(unsignedLengthValidator( > marginsModule->topLE->setValidator(unsignedLengthValidator( > marginsModule->bottomLE->setValidator(unsignedLengthValidator( > marginsModule->innerLE->setValidator(unsignedLengthValidator( > marginsModule->outerLE->setValidator(unsignedLengthValidator( > marginsModule->headsepLE->setValidator(unsignedLengthValidator( > marginsModule->headheightLE->setValidator(unsignedLengthValidator( > marginsModule->footskipLE->setValidator(unsignedLengthValidator( > marginsModule->columnsepLE->setValidator(unsignedLengthValidator( > > all use unsigned validators, so the question is which ones ought to > allow signed quantities. >
Attached is a patch. It seems that everyone in this thread is in favor of allowing negative values where LaTeX allows negative values. The patch does this. Note however that (if I understand correctly the comments at #10095) Jürgen disagrees with such changes. I agree with him partly that it is not a big deal because users can always input negative values manually by inputting custom preamble code. But I am more focused on potential user confusion when transitioning from 2.1.x to 2.2.0; allowing valid input is just a bonus in my opinion. Scott
From 4ad53a855b3e25d90a2a0cc7423bdc87e0b4ebf1 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Tue, 3 May 2016 20:34:17 -0400
Subject: [PATCH] Move some unisigned validators to signed (#10095)
2.1.x allows some document settings to have negative values where
2.2.0rc1 does not (because of the bug fix at 9e166088). If a user of
2.2.0rc1 opens a document from 2.1.x that contains one such negative
value, it will appear as though no change to the document settings
can be saved because 2.2.0rc1 treats the document settings as
invalid immediately on opening the dialog. Further, unless the user
manually goes through each tab they will not see the red text next
to the input that is now considered invalid. This could lead to
confusion for users. One example of such confusion is [1].
The following settings now allow negative values, which is
consistent with 2.1.x. Negative values in these settings do not lead
to LaTeX errors:
- Text Layout tab: the two line edits enabled with "Custom"
- Page Margins tab: all eight line edits
The following settings are not changed by this commit, so they now
(with 2.2.0) do not allow negative values that 2.1.x allowed. This
change makes sense because negative values lead to LaTeX errors in
these cases:
- Page Layout tab: the "Height" and "Width" line edits, which are
enabled when "Custom" is selected
[1]
https://www.mail-archive.com/search?l=mid&q=CAGZ2pgXqf27UaAaQ%3De_wFz1fGTa6Yv0iFyS97qu1C7B5R59irg%40mail.gmail.com
---
src/frontends/qt4/GuiDocument.cpp | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/frontends/qt4/GuiDocument.cpp
b/src/frontends/qt4/GuiDocument.cpp
index 2eedec9..15fb719 100644
--- a/src/frontends/qt4/GuiDocument.cpp
+++ b/src/frontends/qt4/GuiDocument.cpp
@@ -703,9 +703,9 @@ GuiDocument::GuiDocument(GuiView & lv)
textLayoutModule->lspacingLE->setValidator(new QDoubleValidator(
textLayoutModule->lspacingLE));
- textLayoutModule->indentLE->setValidator(unsignedLengthValidator(
+ textLayoutModule->indentLE->setValidator(new LengthValidator(
textLayoutModule->indentLE));
- textLayoutModule->skipLE->setValidator(unsignedGlueLengthValidator(
+ textLayoutModule->skipLE->setValidator(new LengthValidator(
textLayoutModule->skipLE));
textLayoutModule->indentCO->addItem(qt_("Default"));
@@ -966,21 +966,21 @@ GuiDocument::GuiDocument(GuiView & lv)
this, SLOT(change_adaptor()));
connect(marginsModule->columnsepUnit, SIGNAL(activated(int)),
this, SLOT(change_adaptor()));
- marginsModule->topLE->setValidator(unsignedLengthValidator(
+ marginsModule->topLE->setValidator(new LengthValidator(
marginsModule->topLE));
- marginsModule->bottomLE->setValidator(unsignedLengthValidator(
+ marginsModule->bottomLE->setValidator(new LengthValidator(
marginsModule->bottomLE));
- marginsModule->innerLE->setValidator(unsignedLengthValidator(
+ marginsModule->innerLE->setValidator(new LengthValidator(
marginsModule->innerLE));
- marginsModule->outerLE->setValidator(unsignedLengthValidator(
+ marginsModule->outerLE->setValidator(new LengthValidator(
marginsModule->outerLE));
- marginsModule->headsepLE->setValidator(unsignedLengthValidator(
+ marginsModule->headsepLE->setValidator(new LengthValidator(
marginsModule->headsepLE));
- marginsModule->headheightLE->setValidator(unsignedLengthValidator(
+ marginsModule->headheightLE->setValidator(new LengthValidator(
marginsModule->headheightLE));
- marginsModule->footskipLE->setValidator(unsignedLengthValidator(
+ marginsModule->footskipLE->setValidator(new LengthValidator(
marginsModule->footskipLE));
- marginsModule->columnsepLE->setValidator(unsignedLengthValidator(
+ marginsModule->columnsepLE->setValidator(new LengthValidator(
marginsModule->columnsepLE));
bc().addCheckedLineEdit(marginsModule->topLE,
--
2.1.4
signature.asc
Description: PGP signature
