29/04/2013 01:16, Uwe Stöhr:
Am 17.04.2013 13:03, schrieb Jean-Marc Lasgouttes:
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 found now the time to test and your patch does not help. One still
gets "0pt" as length. You can test this by inserting a box, setting its
inner box to makebox and not checking the width and eventually pressing
Apply.
In the View source window you must in this case see \mbox{} if it works.
Did you apply the TWO patches as I wrote? The first patch makes empty
lengths work in general and the second patch uses them in InsetBox
(actually, a small part of the second patch should go to the first one).
Here I do not see any problem with \mbox{}.
I attach again the two patches for simplicity.
JMarc
>From cff706e8401b90f1dc81b2966c6cb498fcfe4c27 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date: Mon, 15 Apr 2013 12:35:11 +0200
Subject: [PATCH 1/2] Improve support for empty lengths
Parse empty string as empty length
Output empty length as empty string when it makes sense (not for LaTeX strings, for example).
---
src/Length.cpp | 13 +++++++++++--
src/lengthcommon.cpp | 10 ++++++++--
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/Length.cpp b/src/Length.cpp
index f8eb920..bb9ec6a 100644
--- a/src/Length.cpp
+++ b/src/Length.cpp
@@ -19,6 +19,7 @@
#include "LyXRC.h"
#include "support/docstream.h"
+#include "support/lassert.h"
#include <sstream>
#include <iomanip>
@@ -67,7 +68,8 @@ void Length::swap(Length & rhs)
string const Length::asString() const
{
ostringstream os;
- os << val_ << unit_name[unit_]; // setw?
+ if (unit_ != UNIT_NONE)
+ os << val_ << unit_name[unit_]; // setw?
return os.str();
}
@@ -75,7 +77,8 @@ string const Length::asString() const
docstring const Length::asDocstring() const
{
odocstringstream os;
- os << val_ << unit_name[unit_]; // setw?
+ if (unit_ != UNIT_NONE)
+ os << val_ << unit_name[unit_]; // setw?
return os.str();
}
@@ -102,6 +105,9 @@ string const Length::asLatexString() const
case PPH:
os << val_ / 100.0 << "\\paperheight";
break;
+ case UNIT_NONE:
+ // One should not try to ouput latex code for an empty length
+ LASSERT(false, break);
default:
os << val_ << unit_name[unit_];
break;
@@ -363,6 +369,9 @@ GlueLength::GlueLength(string const & data)
string const GlueLength::asString() const
{
+ if (len_.empty())
+ return string();
+
ostringstream buffer;
buffer << len_.value();
diff --git a/src/lengthcommon.cpp b/src/lengthcommon.cpp
index 217c22b..72f5bbf 100644
--- a/src/lengthcommon.cpp
+++ b/src/lengthcommon.cpp
@@ -236,8 +236,11 @@ bool isValidGlueLength(string const & data, GlueLength * result)
// forward approach leads to very long, tedious code that would be
// much harder to understand and maintain. (AS)
- if (data.empty())
+ if (data.empty()) {
+ if (result)
+ *result = GlueLength();
return true;
+ }
string buffer = ltrim(data);
// To make isValidGlueLength recognize negative values as
@@ -306,8 +309,11 @@ bool isValidLength(string const & data, Length * result)
// The parser may seem overkill for lengths without
// glue, but since we already have it, using it is
// easier than writing something from scratch.
- if (data.empty())
+ if (data.empty()) {
+ if (result)
+ *result = Length();
return true;
+ }
string buffer = data;
int pattern_index = 0;
--
1.7.0.4
>From e791d046cc876c062692531c67857336cba960ea 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 2/2] 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 | 9 +++----
5 files changed, 46 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..ffe1ded 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();
}
@@ -273,6 +273,7 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
string width_string = params_.width.asLatexString();
bool stdwidth = false;
+ // FIXME: do not test explicitely values of width_string
if (params_.inner_box &&
(width_string.find("1.0\\columnwidth") != string::npos
|| width_string.find("1.0\\textwidth") != string::npos)) {
@@ -316,8 +317,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 +358,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