implement QgsStyleV2::save()

[qgib]

Author Name: Jürgen Fischer (@jef-n)
Original Redmine Issue: 6222
Affected QGIS version: master
Redmine category:symbology

---

QgsStyleV2::save() is disabled (see "qgsstylev2.cpp:338":https://github.com/qgis/Quantum-GIS/blob/master/src/core/symbology-ng/qgsstylev2.cpp#L338) and needs to be implemented.

TestStyleV2::testSaveLoad() is failing because of that (see "testqgsstylev2.cpp:191":https://github.com/qgis/Quantum-GIS/blob/master/tests/src/core/testqgsstylev2.cpp#L191)

See also "![qgis-developer] tests FAILED: 19 - qgis_stylev2test":http://lists.osgeo.org/pipermail/qgis-developer/2012-August/021759.html

---

Related issue(s): #15538 (relates)
Redmine related issue(s): 6229

---

[qgib]

Author Name: Etienne Tourigny (@etiennesky)

---

Martin -

any idea when this could be fixed? I am sort of stuck in the color ramp improvements, because they cannot be saved not restored in current master. Thanks

[qgib]

Author Name: Jürgen Fischer (@jef-n)

---

• assigned_to_id was changed from Martin Dobias to Arunmozhi P

[qgib]

Author Name: Etienne Tourigny (@etiennesky)

---

The patch below solves the creation problem in the UI, but the style test still fails.

QgsStyleV2::save() does nothing, so to save a ramp you need to call QgsStyleV2::saveColorRamp(). However, if save() were implemented (calling save() on symbols and ramps), it would not work with current style manager dialog, because symbols and ramps would be saved twice (which would generate an error on the second save).

My advice would be to save new and modified symbols and ramps when dialog is closed (i.e. in save()), to replicate previous behaviour - although this might be tricky given sql storage. The idea is this should work both in UI and also outside of it, without having to call addSymbol() and saveSymbol() - addSymbol() followed by save() should work as before.

Also, changing existing symbol or color ramp does not work (they are not saved and reloaded when qgis is restarted) - so a bit or work is needed to resolve these 2 issues.

I will file another bug report on that. Thanks

diff --git a/src/gui/symbology-ng/qgsstylev2managerdialog.cpp b/src/gui/symbology-ng/qgsstylev2managerdialog.cpp
index 17d21b9..d8bc2e0 100644
--- a/src/gui/symbology-ng/qgsstylev2managerdialog.cpp
+++ b/src/gui/symbology-ng/qgsstylev2managerdialog.cpp
@@ -437,6 +437,8 @@ QString QgsStyleV2ManagerDialog::addColorRampStatic( QWidget* parent, QgsStyleV2
 
   // add new symbol to style and re-populate the list
   style->addColorRamp( name, ramp );
+  // TODO groups and tags
+  style->saveColorRamp( name, ramp, 0, QStringList() );
   return name;
 }

[qgib]

Author Name: Etienne Tourigny (@etiennesky)

---

• category_id was configured as 83

[qgib]

Author Name: Arunmozhi P (@tecoholic)

---

IMHO, I think we should perhaps do away with the save() altogether and embrace saveSymbol(...) and saveColorRamp(...) instead.

The save() approach was fine when everything was in a single XML file, and was re-written completely whenever the save() was called. That made sense since the entire QgsStyleV2 was written into a file. The same was used for export of symbols as well. But in a SQLite DB, I think applying the same approach is a bit of brute force. We make intelligent usage by saving only what is changed or created.

As for reading/writing in XML files, we can use the importXML(..) and the exportXML(...) functions. Hence, I recommend we re-write the test to include the saveSymbol, saveColorramp, importXML, and exportXML, and drop save() completely.

[qgib]

Author Name: Etienne Tourigny (@etiennesky)

---

Hi, this makes sense. It would be challenging to consider all cases in save().

However, it would be nice to be able to add and save a symbol in one call, something like addSymbol(symbol,name,...,save=true) which calls saveSymbol() if save == true - or saveSymbol() which adds the symbol if it doesn't exist.

What do you think?

[qgib]

Author Name: Etienne Tourigny (@etiennesky)

---

oh and the test does not deal with xml import/export yet, although that would be a good thing to test.

[qgib]

Author Name: Etienne Tourigny (@etiennesky)

---

Partial fixes b145998 (by me) and 447c0d1 (by Arun).

tests now working properly, but leaving this bug open because save() is still not implemented. This is needed to export style.

---

• done_ratio was changed from 0 to 10

[qgib]

Author Name: Paolo Cavallini (@pcav)

---

• fixed_version_id was configured as Version 2.0.0

[qgib]

Author Name: Arunmozhi P (@tecoholic)

---

Etienne Tourigny wrote:

Partial fixes b145998 (by me) and 447c0d1 (by Arun).

tests now working properly, but leaving this bug open because save() is still not implemented. This is needed to export style.

I am wondering what should be done by he save() function. If it has to perform the opposite of what load() does so the load+save test can be carried out, then creating the sqlite db is the way to go. IMHO, sqlite as such is not a good exchange format, hence exportXML and importXML were written. Apart from symbol exchange if there is a requirement for a sqlite file, I think I will write the code for creating a sqlite file. If there is no such requirement, IMO I think, we test load() alone and drop save() altogether. And close this bug too.

[qgib]

Author Name: Jürgen Fischer (@jef-n)

---

• fixed_version_id was changed from Version 2.0.0 to Future Release - Lower Priority

[qgib]

Author Name: Jürgen Fischer (@jef-n)

---

• assigned_to_id removed Arunmozhi P

[qgib]

Author Name: Giovanni Manghi (@gioman)

---

• easy_fix was configured as 0
• regression was configured as 0

[qgib]

Author Name: Giovanni Manghi (@gioman)

---

End of life notice: QGIS 2.18 LTR
*
Source:*
http://blog.qgis.org/2019/03/09/end-of-life-notice-qgis-2-18-ltr/

---

• status_id was changed from Open to Closed
• resolution was changed from to end of life