16/04/2013 23:10, Uwe Stöhr:
LyX now crashes when I use the box dialog. But even if Length() results
in an empty string the result of the parameter reading would still be
"0pt".
As I wrote in a previous message, "What is missing is some code to
detect in the InsetBox dialog that the length value is empty and to use
an empty length in this case". I add some hope that you would continue
the work...
Please find enclosed a patch that should fix the remaining problems
(apply on top of the previous one). I refrained from rewriting large
parts of the code due to your comment below on doing too complicated
things. There is code duplication in GuiBox and the logic is often very
difficult to understand.
I will commit the pair of patches if they work for you. I am not sure
whether this is considered as a format change. I will leave it to
someone else to wrote the lyx2lyx glue in this case.
A remaining bug is that it is not possible anymore to change the width
of a minipage. I leave it to you because I do not understand how
InsetBox works, this endless testing for explicit values of type and
itype parameters just drove me crazy.
In line 303 of GuiBox.cpp we have
Length::UNIT const default_unit = Length::defaultUnit();
and Length::defaultUnit() is "pt".
This is correct and we need a default unit for many cases. So this
cannot be changed.
Actually defaultUnit is either "cm" or "in" and the parameter itself
should just be removed. Work for later...
My solution does indeed not look nice but is with a closer look simpler
than all others that we found so far. So is it really worth it to
implement a new empty length feature? I mean that is much more code than
having in 2 files a special code for en empty length?
Please, Uwe, do not ever consider again to use weird special values to
implement things that look too difficult to do right. The work I did on
empty length will allow to cleanup other code (think about the ugly
support for box height, for example?). I want all these tests for
special values of lengths to just disappear, because they create a lot
of maintenance burden.
Do you really think that the code in InsetBox/GuiBox is not cleaner
after my patch?
JMarc
>From 81f19a1a33efc1479c8fefa0a7846292b79070fa Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date: Wed, 17 Apr 2013 11:30:25 +0200
Subject: [PATCH] Fix empty width support for Box inset
* InsetBox and GuiBox: Use proper empty length instead of the broken -9.99col% trick
* some slight changes to the logic of GuiBox to make sure that values are set as needed.
* lengthToWidget(): handle properly the empty length case. All the other related Qt helpers did it already, it was probably an oversight. Also set the default_unit parameter as optional (not needed in this patch actually, but I got carried away :)
* allow generating LaTeX code for an empty length, since some broken code does that.
---
src/Length.cpp | 4 +--
src/frontends/qt4/GuiBox.cpp | 41 ++++++++++++++++++-------------------
src/frontends/qt4/qt_helpers.cpp | 20 ++++++++++++------
src/frontends/qt4/qt_helpers.h | 12 +++++++---
src/insets/InsetBox.cpp | 8 ++----
5 files changed, 45 insertions(+), 40 deletions(-)
diff --git a/src/Length.cpp b/src/Length.cpp
index bb9ec6a..10cdd29 100644
--- a/src/Length.cpp
+++ b/src/Length.cpp
@@ -19,7 +19,6 @@
#include "LyXRC.h"
#include "support/docstream.h"
-#include "support/lassert.h"
#include <sstream>
#include <iomanip>
@@ -106,8 +105,7 @@ string const Length::asLatexString() const
os << val_ / 100.0 << "\\paperheight";
break;
case UNIT_NONE:
- // One should not try to ouput latex code for an empty length
- LASSERT(false, break);
+ break;
default:
os << val_ << unit_name[unit_];
break;
diff --git a/src/frontends/qt4/GuiBox.cpp b/src/frontends/qt4/GuiBox.cpp
index 657896a..45807ed 100644
--- a/src/frontends/qt4/GuiBox.cpp
+++ b/src/frontends/qt4/GuiBox.cpp
@@ -303,16 +303,18 @@ void GuiBox::paramsToDialog(Inset const * inset)
Length::UNIT const default_unit = Length::defaultUnit();
// the width can only be selected for makebox or framebox
- widthCB->setEnabled(inner_type == "makebox"
- || (type == "Boxed" && !ibox && !pagebreakCB->isChecked()));
- // "-999col%" is the code for no width
- if ((params.width).asString() == "-999col%")
- widthCB->setCheckState(Qt::Unchecked);
- else {
+ widthCB->setEnabled(inner_type == "makebox"
+ || (type == "Boxed"
+ && !ibox && !pagebreakCB->isChecked()));
+ if (params.width.empty()) {
+ widthCB->setChecked(false);
+ lengthToWidgets(widthED, widthUnitsLC,
+ params.width, default_unit);
+ } else {
if (widthCB->isEnabled())
widthCB->setChecked(true);
lengthToWidgets(widthED, widthUnitsLC,
- (params.width).asString(), default_unit);
+ params.width, default_unit);
QString const special = toqstr(params.special);
if (!special.isEmpty() && special != "none")
widthUnitsLC->setCurrentItem(special);
@@ -323,7 +325,7 @@ void GuiBox::paramsToDialog(Inset const * inset)
lengthToWidgets(heightED, heightUnitsLC,
(params.height).asString(), default_unit);
-
+
QString const height_special = toqstr(params.height_special);
if (!height_special.isEmpty() && height_special != "none")
heightUnitsLC->setCurrentItem(height_special);
@@ -367,21 +369,18 @@ docstring GuiBox::dialogToParams() const
widthUnitsLC->itemData(widthUnitsLC->currentIndex()).toString();
QString value = widthED->text();
- if (widthCB->isChecked()) {
- if (ids_spec_.contains(unit) && !isValidLength(fromqstr(value))) {
- params.special = fromqstr(unit);
- // Note: the unit is simply ignored in this case
- params.width = Length(value.toDouble(), Length::IN);
- } else {
- params.special = "none";
- params.width = Length(widgetsToLength(widthED, widthUnitsLC));
- }
- } else {
- if (widthCB->isEnabled()) {
- // use the code "-999col%" for the case that no width was selected
+ if (widthED->isEnabled()) {
+ if (ids_spec_.contains(unit) && !isValidLength(fromqstr(value))) {
+ params.special = fromqstr(unit);
+ // Note: the unit is simply ignored in this case
+ params.width = Length(value.toDouble(), Length::IN);
+ } else {
params.special = "none";
- params.width = Length("-999col%");
+ params.width = Length(widgetsToLength(widthED, widthUnitsLC));
}
+ } else {
+ params.special = "none";
+ params.width = Length();
}
// the height parameter is omitted if the value
diff --git a/src/frontends/qt4/qt_helpers.cpp b/src/frontends/qt4/qt_helpers.cpp
index 17d138f..951cbea 100644
--- a/src/frontends/qt4/qt_helpers.cpp
+++ b/src/frontends/qt4/qt_helpers.cpp
@@ -118,12 +118,18 @@ Length widgetsToLength(QLineEdit const * input, QComboBox const * combo)
void lengthToWidgets(QLineEdit * input, LengthCombo * combo,
- Length const & len, Length::UNIT /*defaultUnit*/)
+ Length const & len, Length::UNIT /*defaultUnit*/)
{
- combo->setCurrentItem(len.unit());
- QLocale loc;
- loc.setNumberOptions(QLocale::OmitGroupSeparator);
- input->setText(loc.toString(Length(len).value()));
+ if (len.empty()) {
+ // no length (UNIT_NONE)
+ combo->setCurrentItem(Length::defaultUnit());
+ input->setText("");
+ } else {
+ combo->setCurrentItem(len.unit());
+ QLocale loc;
+ loc.setNumberOptions(QLocale::OmitGroupSeparator);
+ input->setText(loc.toString(Length(len).value()));
+ }
}
@@ -156,7 +162,7 @@ double widgetToDouble(QLineEdit const * input)
QString const text = input->text();
if (text.isEmpty())
return 0.0;
-
+
return text.trimmed().toDouble();
}
@@ -166,7 +172,7 @@ string widgetToDoubleStr(QLineEdit const * input)
QString const text = input->text();
if (text.isEmpty())
return string();
-
+
return convert<string>(text.trimmed().toDouble());
}
diff --git a/src/frontends/qt4/qt_helpers.h b/src/frontends/qt4/qt_helpers.h
index 753fdbf..54b78be 100644
--- a/src/frontends/qt4/qt_helpers.h
+++ b/src/frontends/qt4/qt_helpers.h
@@ -32,7 +32,7 @@ namespace lyx {
namespace support { class FileName; }
class BufferParams;
-
+
namespace frontend {
class LengthCombo;
@@ -42,13 +42,17 @@ std::string widgetsToLength(QLineEdit const * input, LengthCombo const * combo);
/// method to get a Length from widgets (QComboBox)
Length widgetsToLength(QLineEdit const * input, QComboBox const * combo);
-//FIXME It would be nice if defaultUnit were a default argument
/// method to set widgets from a Length
+//FIXME Remove default_unit argument for the first form. FIXME Change
+// all the code to remove default_unit argument when equal to the
+// default.
void lengthToWidgets(QLineEdit * input, LengthCombo * combo,
-Length const & len, Length::UNIT default_unit);
+ Length const & len,
+ Length::UNIT default_unit = Length::defaultUnit());
/// method to set widgets from a string
void lengthToWidgets(QLineEdit * input, LengthCombo * combo,
-std::string const & len, Length::UNIT default_unit);
+ std::string const & len,
+ Length::UNIT default_unit = Length::defaultUnit());
/// method to set widgets from a docstring
void lengthToWidgets(QLineEdit * input, LengthCombo * combo,
docstring const & len, Length::UNIT default_unit);
diff --git a/src/insets/InsetBox.cpp b/src/insets/InsetBox.cpp
index f610263..fef909a 100644
--- a/src/insets/InsetBox.cpp
+++ b/src/insets/InsetBox.cpp
@@ -162,7 +162,7 @@ void InsetBox::setButtonLabel()
bool InsetBox::hasFixedWidth() const
{
- return from_ascii(params_.width.asLatexString()) != "-9.99\\columnwidth";
+ return !params_.width.empty();
}
@@ -316,8 +316,7 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
os << "\\begin{framed}%\n";
break;
case Boxed:
- // "-999col%" is the code for no width
- if (from_ascii(width_string) != "-9.99\\columnwidth") {
+ if (width_string.empty()) {
os << "\\framebox";
if (!params_.inner_box) {
// Special widths, see usrguide §3.5
@@ -358,8 +357,7 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
if (params_.use_parbox)
os << "\\parbox";
else if (params_.use_makebox) {
- // "-999col%" is the code for no width
- if (from_ascii(width_string) != "-9.99\\columnwidth") {
+ if (!width_string.empty()) {
os << "\\makebox";
// FIXME UNICODE
// output the width and horizontal position
--
1.7.0.4
