Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change style file extension from QML to QLP #4025

Closed
wants to merge 2 commits into from
Closed

change style file extension from QML to QLP #4025

wants to merge 2 commits into from

Conversation

alexbruy
Copy link
Contributor

This fixes overlap with QtQuick/QtQML files and also reflects that this is more than just style, as now it contains labeling settings, widgets settings etc.

I only chaged corresponding strings and some variable names. Most of the variables and names of the fields in the databases left unchanged (e.g. when saving style to database). Not sure if we should change them too.

This fixes overlap with QtQuick/QtQML files and also reflects
that this is more than just style, as now it contains labeling
settings, widgets etc.
@@ -1762,14 +1762,10 @@ void QgsRasterLayerProperties::loadStyle_clicked()
this,
tr( "Load layer properties from style file" ),
lastUsedDir,
tr( "QGIS Layer Style File" ) + " (*.qml)" );
tr( "QGIS Layer Properties File" ) + " (*.qlp);;" + tr( "QGIS Layer Style File" ) + " (*.qml)" );
Copy link
Member

@m-kuhn m-kuhn Jan 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rephrase this to "QGIS Layer Style File (deprecated)" to reduce the confusion about "what's the difference between the two?"?
Is the double ;; on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double ;; is on purpose this is the syntax for filters in Qt file dialogs.

Copy link
Member

@m-kuhn m-kuhn Jan 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or "QGIS 2 Layer Style Files"?

@m-kuhn
Copy link
Member

m-kuhn commented Jan 19, 2017

It would be nice to rename database stuff 2. I'm not involved enough to see if we can do automatic updating of this?

@alexbruy
Copy link
Contributor Author

As I can see the only thing that needs updating in the database part is the name of the field styleQML and some internal variables. For example https://github.com/qgis/QGIS/blob/master/src/providers/postgres/qgspostgresprovider.cpp#L4403 or https://github.com/qgis/QGIS/blob/master/src/providers/postgres/qgspostgresprovider.cpp#L4448

@m-kuhn
Copy link
Member

m-kuhn commented Jan 19, 2017

Do you think we should also ship an ALTER TABLE layer_styles RENAME column script? Detect if the user is still running a 2.x version and propose to update?

@jgrocha
Copy link
Member

jgrocha commented Jan 21, 2017

Hi,
If we update the styleqml column name, we might invalidate an environment where users are using QGIS 2.x and QGIS 3.x on top of the same database. If the table is created with QGIS 3.x, a QGIS 2.x user would have to delete the table manually, to recreate it for 2.x
At least, we should warn the users of this potential inconsistency.
We might end with much more noise if we change the column name, rather than keep it as it is.

@m-kuhn
Copy link
Member

m-kuhn commented Feb 3, 2017

we might invalidate an environment where users are using QGIS 2.x and QGIS 3.x on top of the same database

I'm not sure the contents (style XML) will be compatible in every case.
So I imagine we can even end up with better UX if we have two different columns for 2.x styles and 3.x style?

@jgrocha
Copy link
Member

jgrocha commented Feb 5, 2017

@m-kuhn Adding the styleqlp column if necessary is not difficult to implement and the user will not notice if the style comes from the styleqml or the styleqlp column. This can support an hybrid environment with QGIS 2.x and 3.x versions running on top of the same database.

@alexbruy
Copy link
Contributor Author

alexbruy commented Feb 6, 2017

As QGIS 3 will introduce a lot of API breaks, I think we can also break table schema. Moreover styles definition (and other parts of the QML/QLP file) may also change during QGIS 3.0 development.

@NathanW2
Copy link
Member

NathanW2 commented Feb 6, 2017 via email

@alexbruy
Copy link
Contributor Author

alexbruy commented Feb 8, 2017

Well, I can add another column to the layer_styles table when creating it from scratch and some logic to alter this table if it is already present in a database. But at some point we anyway should drop QML column from it otherwise we will end up with a huge table with columns for each QGIS version, because same DB may be used by QGIS 2, QGIS 3, QGIS 4....

@NathanW2
Copy link
Member

NathanW2 commented Feb 8, 2017 via email

@m-kuhn
Copy link
Member

m-kuhn commented Feb 13, 2017

I can add another column to the layer_styles table when creating it from scratch and some logic to alter this table if it is already present in a database.

That sounds good to me.

Apart from the database pollution, which only happens when migrating and not when creating from scratch, I think that sound like a sane approach?

@m-kuhn
Copy link
Member

m-kuhn commented Aug 18, 2017

@alexbruy any news on this, it would be a nice change for 3.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants