Bug report #6222
|Affected QGIS version:||master||Regression?:||No|
|Operating System:||Easy fix?:||No|
|Pull Request or Patch supplied:||No||Resolution:||end of life|
|Crashes QGIS or corrupts data:||No||Copied to github as #:||15533|
QgsStyleV2::save() is disabled (see qgsstylev2.cpp:338) and needs to be implemented.
TestStyleV2::testSaveLoad() is failing because of that (see testqgsstylev2.cpp:191)
#3 Updated by Etienne Tourigny over 8 years ago
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; }
#5 Updated by Arunmozhi P over 8 years ago
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.
#6 Updated by Etienne Tourigny over 8 years ago
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?
#10 Updated by Arunmozhi P about 8 years ago
Etienne Tourigny wrote:
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.
#14 Updated by Giovanni Manghi over 1 year ago
- Status changed from Open to Closed
- Resolution set to end of life
End of life notice: QGIS 2.18 LTR