From 857274be46b95828266d51bc7f95ffa5f3c63491 Mon Sep 17 00:00:00 2001 From: Anunay Jain Date: Sat, 22 Mar 2025 22:03:54 +0530 Subject: [PATCH 1/2] Deprecate isValid() in preferences.cpp isValid() was misleading as all it checked was if the value existed. It has been deprecated in favour of of a renamed function isSet(). Move all instances of isValid() to use isSet() instead (preferences.cpp) isValid*() functions are not used as the cost of using them is same as the cost incurred by the get*() functions, and the get* functions check the validity sunsequently anyway. --- src/actions/actions-canvas-snapping.cpp | 2 +- src/live_effects/effect.cpp | 6 +-- src/live_effects/lpe-roughen.cpp | 2 +- src/preferences.cpp | 21 +++++---- src/preferences.h | 22 +++++++-- testfiles/src/preferences-test.cpp | 60 ++++++++++++++++++++----- 6 files changed, 85 insertions(+), 28 deletions(-) diff --git a/src/actions/actions-canvas-snapping.cpp b/src/actions/actions-canvas-snapping.cpp index f8b7a94df1..da38807451 100644 --- a/src/actions/actions-canvas-snapping.cpp +++ b/src/actions/actions-canvas-snapping.cpp @@ -131,7 +131,7 @@ SnapPreferences& get_snapping_preferences() { } auto simple = prefs->getEntry("/toolbox/simplesnap"); - if (!simple.isValid()) { + if (!simple.isSet()) { // first time up after creating preferences; apply "simple" snapping defaults prefs->setBool(simple.getPath(), true); transition_to_simple_snapping(); diff --git a/src/live_effects/effect.cpp b/src/live_effects/effect.cpp index 38127a421d..cef2dbfb95 100644 --- a/src/live_effects/effect.cpp +++ b/src/live_effects/effect.cpp @@ -1677,7 +1677,7 @@ Effect::readallParameters(Inkscape::XML::Node const* repr) (Glib::ustring)LPETypeConverter.get_key(effectType()).c_str() + (Glib::ustring)"/" + (Glib::ustring)key; - bool valid = prefs->getEntry(pref_path).isValid(); + bool valid = prefs->getEntry(pref_path).isSet(); if(valid){ param->param_update_default(prefs->getString(pref_path).c_str()); } else { @@ -1874,7 +1874,7 @@ Effect::hasDefaultParameters() pref_path += effectkey; pref_path +="/"; pref_path += key; - if (prefs->getEntry(pref_path).isValid()) { + if (prefs->getEntry(pref_path).isSet()) { return true; } ++it; @@ -1926,7 +1926,7 @@ void Effect::unsetDefaultParam(Glib::ustring pref_path,Parameter *param) Glib::ustring value = param->param_getSVGValue(); Glib::ustring defvalue = param->param_getDefaultSVGValue(); Inkscape::Preferences *prefs = Inkscape::Preferences::get(); - if (prefs->getEntry(pref_path).isValid()) { + if (prefs->getEntry(pref_path).isSet()) { prefs->remove(pref_path); } } diff --git a/src/live_effects/lpe-roughen.cpp b/src/live_effects/lpe-roughen.cpp index 2dda198864..31e00ac328 100644 --- a/src/live_effects/lpe-roughen.cpp +++ b/src/live_effects/lpe-roughen.cpp @@ -101,7 +101,7 @@ void LPERoughen::doOnApply(SPLPEItem const *lpeitem) auto const pref_path = Glib::ustring::compose("/live_effects/%1/%2", LPETypeConverter.get_key(effectType()), param->param_key); - if (prefs->getEntry(pref_path).isValid()) continue; + if (prefs->getEntry(pref_path).isSet()) continue; if (param->param_key == "max_segment_size") { auto const minor = std::min(bbox->width(), bbox->height()); diff --git a/src/preferences.cpp b/src/preferences.cpp index 4c825ad7b1..c86a38161d 100644 --- a/src/preferences.cpp +++ b/src/preferences.cpp @@ -772,12 +772,13 @@ void Preferences::_setRawValue(Glib::ustring const &path, Glib::ustring const &v node->setAttribute(attr_key, value); } + // The Entry::get* methods convert the preference string from the XML file back to the original value. // The conversions here are the inverse of Preferences::set*. bool Preferences::Entry::getBool(bool def) const { - if (!isValid()) { + if (!isSet()) { return def; } if (cached_bool) { @@ -801,7 +802,7 @@ bool Preferences::Entry::getBool(bool def) const Colors::Color Preferences::Entry::getColor(std::string const &def) const { - if (isValid()) { + if (isSet()) { // Note: we don't cache the resulting Color object // because this function is called rarely and therefore not performance-relevant // (exemplary Inkscape startup: 40 calls to getColor vs. 10k calls to getBool()) @@ -818,7 +819,7 @@ Colors::Color Preferences::Entry::getColor(std::string const &def) const int Preferences::Entry::getInt(int def) const { - if (!isValid()) { + if (!isSet()) { return def; } if (cached_int) { @@ -828,8 +829,10 @@ int Preferences::Entry::getInt(int def) const // .raw() is only for performance reasons (std::string comparison is faster than Glib::ustr) auto const &s = _value.value().raw(); if (s == "true") { + g_warning("Integer preference value is set as true, treating it as 1: %s", _pref_path.c_str()); value_int = 1; } else if (s == "false") { + g_warning("Integer preference value is set as false, treating it as 0: %s", _pref_path.c_str()); value_int = 0; } else { int val = 0; @@ -860,7 +863,7 @@ int Preferences::Entry::getIntLimited(int def, int min, int max) const unsigned int Preferences::Entry::getUInt(unsigned int def) const { - if (!isValid()) { + if (!isSet()) { return def; } if (cached_uint) { @@ -902,7 +905,7 @@ double Preferences::Entry::_getDoubleAssumeExisting() const double Preferences::Entry::getDouble(double def, Glib::ustring const &requested_unit) const { - if (!isValid()) { + if (!isSet()) { return def; } @@ -927,7 +930,7 @@ Glib::ustring Preferences::Entry::getString(Glib::ustring const &def) const Glib::ustring Preferences::Entry::getUnit() const { - if (!isValid()) { + if (!isSet()) { return ""; } if (cached_unit) { @@ -945,7 +948,7 @@ Glib::ustring Preferences::Entry::getUnit() const } if (end_index == 0) { [[unlikely]]; - // isValid()==true but the string + // isSet() == true but the string is: // - is empty // - or does not start with a numeric value // - or the number is out of range (double over/underflow) @@ -963,7 +966,7 @@ Glib::ustring Preferences::Entry::getUnit() const SPCSSAttr *Preferences::Entry::getStyle() const { - if (!isValid()) { + if (!isSet()) { return sp_repr_css_attr_new(); } // Note: the resulting style object is not cached. @@ -979,7 +982,7 @@ SPCSSAttr *Preferences::Entry::getInheritedStyle() const // This method is quite "dirty". We ignore whatever is stored this Entry // and just get the style from Preferences. // A more beautiful solution would need major refactoring of Entry and Preferences. - if (!isValid()) { + if (!isSet()) { return sp_repr_css_attr_new(); } diff --git a/src/preferences.h b/src/preferences.h index 20d4584e49..e52f2b0299 100644 --- a/src/preferences.h +++ b/src/preferences.h @@ -172,14 +172,28 @@ public: Entry(Entry const &other) = default; /** - * Check whether the received entry is valid. + * Check whether the received entry is set. * This means that the requested preference path exists. * * If not, then the get...() functions will return the default value. * - * @return True if the entry valid. + * @return True if the entry exists. */ - bool isValid() const { return _value.has_value(); } + bool isSet() const { return _value.has_value(); } + + /** + * @deprecated This function is deprecated because its name was misleading from its purpose. + * Use `isSet()` instead to check whether the preference entry exists. + * + * Check whether the received entry is set. + * This means that the requested preference path exists. + * + * If not, then the get...() functions will return the default value. + * + * @return True if the entry exists. + */ + [[deprecated("Use isSet() instead")]] + bool isValid() const { return isSet(); } /** * Interpret the preference as a Boolean value. @@ -378,7 +392,7 @@ public: * @param pref_path Path to preference to check. */ bool hasPref(Glib::ustring const &pref_path) { - return getEntry(pref_path).isValid(); + return getEntry(pref_path).isSet(); } /** diff --git a/testfiles/src/preferences-test.cpp b/testfiles/src/preferences-test.cpp index 997cb5593b..240afd89f1 100644 --- a/testfiles/src/preferences-test.cpp +++ b/testfiles/src/preferences-test.cpp @@ -26,11 +26,11 @@ public: { value = val.getInt(); value_str = val.getString(); - value_valid = val.isValid(); + value_set = val.isSet(); } int value; std::string value_str; - bool value_valid; + bool value_set; }; class PreferencesTest : public ::testing::Test @@ -158,6 +158,46 @@ TEST_F(PreferencesTest, testColorDefaultReturn) ASSERT_EQ(prefs->getColor("/test/colorvalueNonExistent", "green"), green); } +TEST_F(PreferencesTest, testIsValidBool) +{ + prefs->setBool("/test/boolvalue", true); + ASSERT_TRUE(prefs->getEntry("/test/boolvalue").isValidBool()); + prefs->setString("/test/boolvalue", "invalid"); + ASSERT_FALSE(prefs->getEntry("/test/boolvalue").isValidBool()); +} + +TEST_F(PreferencesTest, testIsValidInt) +{ + prefs->setInt("/test/intvalue", 123); + ASSERT_TRUE(prefs->getEntry("/test/intvalue").isValidInt()); + prefs->setString("/test/intvalue", "invalid"); + ASSERT_FALSE(prefs->getEntry("/test/intvalue").isValidInt()); +} + +TEST_F(PreferencesTest, testIsValidUInt) +{ + prefs->setUInt("/test/uintvalue", 123u); + ASSERT_TRUE(prefs->getEntry("/test/uintvalue").isValidUInt()); + prefs->setString("/test/uintvalue", "-123"); + ASSERT_FALSE(prefs->getEntry("/test/uintvalue").isValidUInt()); +} + +TEST_F(PreferencesTest, testIsValidDouble) +{ + prefs->setDouble("/test/doublevalue", 123.456); + ASSERT_TRUE(prefs->getEntry("/test/doublevalue").isValidDouble()); + prefs->setString("/test/doublevalue", "invalid"); + ASSERT_FALSE(prefs->getEntry("/test/doublevalue").isValidDouble()); +} + +TEST_F(PreferencesTest, testIsValidColor) +{ + prefs->setColor("/test/colorvalue", Inkscape::Colors::Color::parse("blue").value()); + ASSERT_TRUE(prefs->getEntry("/test/colorvalue").isValidColor()); + prefs->setString("/test/colorvalue", "invalid"); + ASSERT_FALSE(prefs->getEntry("/test/colorvalue").isValidColor()); +} + TEST_F(PreferencesTest, testKeyObserverNotification) { Glib::ustring const path = "/some/random/path"; @@ -169,7 +209,7 @@ TEST_F(PreferencesTest, testKeyObserverNotification) prefs->addObserver(obs); prefs->setInt(path, 10); ASSERT_EQ(obs.value, 10); - ASSERT_TRUE(obs.value_valid); + ASSERT_TRUE(obs.value_set); prefs->setInt("/some/other/random/path", 42); ASSERT_EQ(obs.value, 10); // value should not change @@ -193,17 +233,17 @@ TEST_F(PreferencesTest, testKeyObserverNotificationAddRemove) // value is added (set for the first time) prefs->setInt(path, 10); ASSERT_EQ(obs.value, 10); - ASSERT_TRUE(obs.value_valid); + ASSERT_TRUE(obs.value_set); // set to empty string --> observer should still receive a valid (but empty) entry prefs->setString(path, ""); ASSERT_EQ(obs.value_str, ""); ASSERT_EQ(obs.value, 0); // fallback value for int - ASSERT_TRUE(obs.value_valid); + ASSERT_TRUE(obs.value_set); // remove preference --> observer should still receive a non-existing entry (isValid==false) prefs->remove(path); - ASSERT_FALSE(obs.value_valid); + ASSERT_FALSE(obs.value_set); // Remove key and then set again. // In this case the observer may stop working. @@ -234,12 +274,12 @@ TEST_F(PreferencesTest, testEntryObserverNotificationAddRemove) prefs->addObserver(obs); prefs->setInt(path, 10); - ASSERT_TRUE(obs.value_valid); + ASSERT_TRUE(obs.value_set); ASSERT_EQ(obs.value, 10); // empty string (not the same as removed) prefs->setString(path, ""); - ASSERT_TRUE(obs.value_valid); + ASSERT_TRUE(obs.value_set); ASSERT_EQ(obs.value_str, ""); ASSERT_EQ(obs.value, 0); // fallback value for int conversion @@ -247,7 +287,7 @@ TEST_F(PreferencesTest, testEntryObserverNotificationAddRemove) ASSERT_EQ(obs.value, 15); prefs->remove(path); - ASSERT_FALSE(obs.value_valid); + ASSERT_FALSE(obs.value_set); // Note: Here we are re-adding a removed preference. // The observer still works, but would also be allowed to fail, see Preferences::addObserver. @@ -288,7 +328,7 @@ TEST_F(PreferencesTest, testPreferencesEntryMethods) { prefs->setInt("/test/prefentry", 100); Inkscape::Preferences::Entry val = prefs->getEntry("/test/prefentry"); - ASSERT_TRUE(val.isValid()); + ASSERT_TRUE(val.isSet()); ASSERT_EQ(val.getPath(), "/test/prefentry"); ASSERT_EQ(val.getEntryName(), "prefentry"); ASSERT_EQ(val.getInt(), 100); -- GitLab From d54344c5be89ef0a7a87de2e838204ec14011f4a Mon Sep 17 00:00:00 2001 From: Anunay Jain Date: Sun, 23 Mar 2025 00:48:12 +0530 Subject: [PATCH 2/2] Add isValid*() methods to preferences.cpp, Remove isValid() Adds isValidBool(), isValidInt(), isValidUInt(), isValidDouble(), isValidColor(), isValidString (which is just isSet()) methods to allow checking if a stored preference is a valid value. Unlike get* methods, these methods won't return a default value if the preferences are wrongly set. Fixes: https://gitlab.com/inkscape/inkscape/-/issues/295 --- src/live_effects/lpe-jointype.cpp | 2 +- src/live_effects/lpe-roughen.cpp | 3 +- src/live_effects/lpe-taperstroke.cpp | 2 +- src/preferences.cpp | 123 +++++++++++++++++++++++++ src/preferences.h | 47 +++++++--- src/ui/dialog/inkscape-preferences.cpp | 6 +- src/ui/themes.cpp | 2 +- src/ui/tools/pen-tool.cpp | 5 +- src/ui/tools/pencil-tool.cpp | 12 +-- src/ui/widget/desktop-widget.cpp | 2 +- testfiles/src/preferences-test.cpp | 5 +- 11 files changed, 179 insertions(+), 30 deletions(-) diff --git a/src/live_effects/lpe-jointype.cpp b/src/live_effects/lpe-jointype.cpp index 55062eb236..084035e9b0 100644 --- a/src/live_effects/lpe-jointype.cpp +++ b/src/live_effects/lpe-jointype.cpp @@ -96,7 +96,7 @@ void LPEJoinType::doOnApply(SPLPEItem const* lpeitem) (Glib::ustring)"/" + (Glib::ustring)"line_width"; - bool valid = prefs->getEntry(pref_path).isValid(); + bool valid = prefs->getEntry(pref_path).isValidDouble(); if (!valid) { line_width.param_set_value(width); diff --git a/src/live_effects/lpe-roughen.cpp b/src/live_effects/lpe-roughen.cpp index 31e00ac328..9d44748b9c 100644 --- a/src/live_effects/lpe-roughen.cpp +++ b/src/live_effects/lpe-roughen.cpp @@ -101,7 +101,8 @@ void LPERoughen::doOnApply(SPLPEItem const *lpeitem) auto const pref_path = Glib::ustring::compose("/live_effects/%1/%2", LPETypeConverter.get_key(effectType()), param->param_key); - if (prefs->getEntry(pref_path).isSet()) continue; + if (prefs->getEntry(pref_path).isSet()) + continue; if (param->param_key == "max_segment_size") { auto const minor = std::min(bbox->width(), bbox->height()); diff --git a/src/live_effects/lpe-taperstroke.cpp b/src/live_effects/lpe-taperstroke.cpp index 94ce67de12..25b19194fe 100644 --- a/src/live_effects/lpe-taperstroke.cpp +++ b/src/live_effects/lpe-taperstroke.cpp @@ -184,7 +184,7 @@ void LPETaperStroke::doOnApply(SPLPEItem const* lpeitem) (Glib::ustring)"/" + (Glib::ustring)"stroke_width"; - bool valid = prefs->getEntry(pref_path).isValid(); + bool valid = prefs->getEntry(pref_path).isValidDouble(); if (!valid) { line_width.param_set_value(width); diff --git a/src/preferences.cpp b/src/preferences.cpp index c86a38161d..914a0bbb5a 100644 --- a/src/preferences.cpp +++ b/src/preferences.cpp @@ -772,6 +772,129 @@ void Preferences::_setRawValue(Glib::ustring const &path, Glib::ustring const &v node->setAttribute(attr_key, value); } +// The Entry::isValid* methods check if the preference exists, and then verify if the data would be +// correctly converted to the requested type. + +bool Preferences::Entry::isValidBool() const +{ + if (!isSet()) { + return false; + } + auto const &s = _value.value().raw(); + // format is currently "0"/"1", may change to "true"/"false" in the future + // see Preferences::setBool() + return (s == "1" || s == "0" || s == "true" || s == "false"); +} + +bool Preferences::Entry::isValidInt() const +{ + if (!isSet()) { + return false; + } + + auto const &s = _value.value().raw(); + + // true, false are treated as 1, 0 by getInt(), even though it's not entirely appropriate + // we're gonna treat them as valid integers here + if (s == "true" || s == "false") { + // warn that we're treating "true" and "false" as integers + g_warning("Integer preference value are set as boolean: '%s', treating it as %d: %s", s.c_str(), + s == "true" ? 1 : 0, _pref_path.c_str()); + return true; + } + + errno = 0; + + const char* cstr = s.c_str(); + char* endPtr = nullptr; + long value = strtol(cstr, &endPtr, 0); + if (endPtr == cstr) { + // no valid number found + return false; + } + // checking for overflow is unecessary because long is the same size + // as int on all modern platforms, this is somewhat pedantic + if (errno == ERANGE || value < INT_MIN || value > INT_MAX) { + return false; // overflow + } + + // getInt() will also happily retrieve unsigned integers as overflow them + // However we have a getUInt() method for that, we're gonna therefore + // treat them as invalid. + + return true; +} + +bool Preferences::Entry::isValidUInt() const +{ + if (!isSet()) { + return false; + } + + auto const &s = _value.value().raw(); + + errno = 0; + const char* cstr = s.c_str(); + char* end_ptr = nullptr; + unsigned long value = strtoul(cstr, &end_ptr, 0); + if (end_ptr == cstr) { + return false; + } + if (errno == ERANGE || value > UINT_MAX) { + return false; // overflow + } + + return true; +} + +bool Preferences::Entry::isValidDouble() const +{ + if (!isSet()) { + return false; + } + + auto const &value_str = _value.value().raw(); + std::string::size_type end_index = 0; + + try { + Glib::Ascii::strtod(value_str, end_index, 0); + } catch (std::runtime_error const &e) { + return false; + } + + if(end_index == 0) { + return false; // failed to read anything numeric + } + + // extract the unit if any, and check if it's a valid unit + auto unit = value_str.substr(end_index); + if(!unit.empty()) { + return Util::UnitTable::get().hasUnit(unit); + } + + return true; +} + +bool Preferences::Entry::isConvertibleTo(Glib::ustring const &type) const +{ + auto from = getUnit(); + if (!from.empty()) { + auto to = Util::UnitTable::get().getUnit(type); + return to->compatibleWith(from); + } + + // if the unit is empty + return false; +} + +bool Preferences::Entry::isValidColor() const +{ + if (!isSet()) { + return false; + } + + return Colors::Color::parse(_value.value().raw()).has_value(); +} // The Entry::get* methods convert the preference string from the XML file back to the original value. // The conversions here are the inverse of Preferences::set*. diff --git a/src/preferences.h b/src/preferences.h index e52f2b0299..6c58e5fd6f 100644 --- a/src/preferences.h +++ b/src/preferences.h @@ -182,18 +182,43 @@ public: bool isSet() const { return _value.has_value(); } /** - * @deprecated This function is deprecated because its name was misleading from its purpose. - * Use `isSet()` instead to check whether the preference entry exists. - * - * Check whether the received entry is set. - * This means that the requested preference path exists. - * - * If not, then the get...() functions will return the default value. - * - * @return True if the entry exists. + * Check if the preference value can be interpreted as a Boolean. + */ + bool isValidBool() const; + + /** + * Check if the preference value can be interpreted as an integer without any overflow. + * It also treats true and false as valid values. + */ + bool isValidInt() const; + + /** + * Check if the preference value can be interpreted as an unsigned integer. + */ + bool isValidUInt() const; + + /** + * Check if the preference value can be interpreted as a floating point value. + * This will also return true if the value is a valid unit. (eg: "1.0pt") + */ + bool isValidDouble() const; + + /** + * Check if the preference value can be converted to a particular unit. + */ + + bool isConvertibleTo(Glib::ustring const &unit) const; + + /** + * Check if the preference value can be interpreted as a color. + */ + bool isValidColor() const; + + /** + * Check if the preference value is a valid String + * @return True if the preference is set, as all data is stored as strings. */ - [[deprecated("Use isSet() instead")]] - bool isValid() const { return isSet(); } + bool isValidString() const { return isSet(); }; /** * Interpret the preference as a Boolean value. diff --git a/src/ui/dialog/inkscape-preferences.cpp b/src/ui/dialog/inkscape-preferences.cpp index 4b76035e1f..381a5d5425 100644 --- a/src/ui/dialog/inkscape-preferences.cpp +++ b/src/ui/dialog/inkscape-preferences.cpp @@ -1230,7 +1230,7 @@ void InkscapePreferences::resetIconsColors(bool themechange) auto doChangeIconsColors = false; if (prefs->getBool("/theme/symbolicDefaultBaseColors", true) || - !prefs->getEntry("/theme/" + themeiconname + "/symbolicBaseColor").isValid()) { + !prefs->getEntry("/theme/" + themeiconname + "/symbolicBaseColor").isValidUInt()) { auto const display = Gdk::Display::get_default(); if (INKSCAPE.themecontext->getColorizeProvider()) { Gtk::StyleProvider::remove_provider_for_display(display, INKSCAPE.themecontext->getColorizeProvider()); @@ -1343,7 +1343,7 @@ void InkscapePreferences::toggleSymbolic() _symbolic_highlight_colors.set_sensitive(true); Glib::ustring themeiconname = prefs->getString("/theme/iconTheme", prefs->getString("/theme/defaultIconTheme", "")); if (prefs->getBool("/theme/symbolicDefaultColors", true) || - !prefs->getEntry("/theme/" + themeiconname + "/symbolicBaseColor").isValid()) { + !prefs->getEntry("/theme/" + themeiconname + "/symbolicBaseColor").isValidUInt()) { resetIconsColors(); } else { changeIconsColors(); @@ -1490,7 +1490,7 @@ void InkscapePreferences::symbolicThemeCheck() if (symbolic) { if (prefs->getBool("/theme/symbolicDefaultHighColors", true) || prefs->getBool("/theme/symbolicDefaultBaseColors", true) || - !prefs->getEntry("/theme/" + themeiconname + "/symbolicBaseColor").isValid()) { + !prefs->getEntry("/theme/" + themeiconname + "/symbolicBaseColor").isValidUInt()) { resetIconsColors(); } else { changeIconsColors(); diff --git a/src/ui/themes.cpp b/src/ui/themes.cpp index d8dcb6a91c..3d072cc19b 100644 --- a/src/ui/themes.cpp +++ b/src/ui/themes.cpp @@ -414,7 +414,7 @@ void ThemeContext::add_gtk_css(bool only_providers, bool cached) // note: ideally we should remove the callback during destruction, but ThemeContext is never deleted prefs->addObserver(*_spinbutton_observer); // establish default value, so both this setting here and checkbox in preferences are in sync - if (!prefs->getEntry(_spinbutton_observer->observed_path).isValid()) { + if (!prefs->getEntry(_spinbutton_observer->observed_path).isValidBool()) { prefs->setBool(_spinbutton_observer->observed_path, true); } _spinbutton_observer->notify(prefs->getEntry(_spinbutton_observer->observed_path)); diff --git a/src/ui/tools/pen-tool.cpp b/src/ui/tools/pen-tool.cpp index ec411c5a6a..4c4807a677 100644 --- a/src/ui/tools/pen-tool.cpp +++ b/src/ui/tools/pen-tool.cpp @@ -1622,11 +1622,8 @@ void PenTool::_bsplineSpiroBuild() if (bspline) { Inkscape::Preferences *prefs = Inkscape::Preferences::get(); Geom::PathVector hp; - bool uniform = false; Glib::ustring pref_path = "/live_effects/bspline/uniform"; - if (prefs->getEntry(pref_path).isValid()) { - uniform = prefs->getString(pref_path) == "true"; - } + bool uniform = prefs->getBool(pref_path, false); LivePathEffect::sp_bspline_do_effect(curve, 0, hp, uniform); } else { LivePathEffect::sp_spiro_do_effect(curve); diff --git a/src/ui/tools/pencil-tool.cpp b/src/ui/tools/pencil-tool.cpp index c3813ed1c5..e24811260f 100644 --- a/src/ui/tools/pencil-tool.cpp +++ b/src/ui/tools/pencil-tool.cpp @@ -713,17 +713,17 @@ void PencilTool::addPowerStrokePencil() if (simplify) { sp_lpe_item_enable_path_effects(lpeitem, false); Glib::ustring pref_path = "/live_effects/simplify/smooth_angles"; - bool valid = prefs->getEntry(pref_path).isValid(); + bool valid = prefs->getEntry(pref_path).isValidDouble(); if (!valid) { lpe->getRepr()->setAttribute("smooth_angles", "0"); } pref_path = "/live_effects/simplify/helper_size"; - valid = prefs->getEntry(pref_path).isValid(); + valid = prefs->getEntry(pref_path).isValidDouble(); if (!valid) { lpe->getRepr()->setAttribute("helper_size", "0"); } pref_path = "/live_effects/simplify/step"; - valid = prefs->getEntry(pref_path).isValid(); + valid = prefs->getEntry(pref_path).isValidDouble(); if (!valid) { lpe->getRepr()->setAttribute("step", "1"); } @@ -749,17 +749,17 @@ void PencilTool::addPowerStrokePencil() if (pspreview) { sp_lpe_item_enable_path_effects(lpeitem, false); Glib::ustring pref_path = "/live_effects/powerstroke/interpolator_type"; - bool valid = prefs->getEntry(pref_path).isValid(); + bool valid = prefs->getEntry(pref_path).isValidString(); if (!valid) { pspreview->getRepr()->setAttribute("interpolator_type", "CentripetalCatmullRom"); } pref_path = "/live_effects/powerstroke/linejoin_type"; - valid = prefs->getEntry(pref_path).isValid(); + valid = prefs->getEntry(pref_path).isValidString(); if (!valid) { pspreview->getRepr()->setAttribute("linejoin_type", "spiro"); } pref_path = "/live_effects/powerstroke/interpolator_beta"; - valid = prefs->getEntry(pref_path).isValid(); + valid = prefs->getEntry(pref_path).isValidDouble(); if (!valid) { pspreview->getRepr()->setAttribute("interpolator_beta", "0.75"); } diff --git a/src/ui/widget/desktop-widget.cpp b/src/ui/widget/desktop-widget.cpp index cb380713f7..a9bcf91461 100644 --- a/src/ui/widget/desktop-widget.cpp +++ b/src/ui/widget/desktop-widget.cpp @@ -146,7 +146,7 @@ SPDesktopWidget::SPDesktopWidget(InkscapeWindow *inkscape_window) repack_snaptoolbar(); auto tbox_width = prefs->getEntry("/toolbox/tools/width"); - if (tbox_width.isValid()) { + if (tbox_width.isSet()) { _tbbox->set_position(tbox_width.getIntLimited(32, 8, 500)); } diff --git a/testfiles/src/preferences-test.cpp b/testfiles/src/preferences-test.cpp index 240afd89f1..c799526012 100644 --- a/testfiles/src/preferences-test.cpp +++ b/testfiles/src/preferences-test.cpp @@ -194,7 +194,10 @@ TEST_F(PreferencesTest, testIsValidColor) { prefs->setColor("/test/colorvalue", Inkscape::Colors::Color::parse("blue").value()); ASSERT_TRUE(prefs->getEntry("/test/colorvalue").isValidColor()); - prefs->setString("/test/colorvalue", "invalid"); + prefs->setString("/test/colorvalue", "#2E3436ff"); + ASSERT_TRUE(prefs->getEntry("/test/colorvalue").isValidColor()); + + prefs->setString("/test/colorvalue", "22px"); ASSERT_FALSE(prefs->getEntry("/test/colorvalue").isValidColor()); } -- GitLab