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

return the default style first in the list #7172

Merged
merged 1 commit into from
Jun 21, 2018

Conversation

Gustry
Copy link
Contributor

@Gustry Gustry commented Jun 4, 2018

Description

iface.activeLayer().listStylesInDatabase()
(3, ['60', '122', '120', '129', '128', ..............

With this fix, I know that 60 is the default style, 122 and 120 are other related styles, 129, 128 etc are non related styles.

It's the only way in the API to know the default style.

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

  • Commit messages are descriptive and explain the rationale for changes
  • Commits which fix bugs include fixes #11111 in the commit message next to the description
  • Commits which add new features are tagged with [FEATURE] in the commit message
  • Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and contain sufficient information in the commit message to be documented
  • I have read the QGIS Coding Standards and this PR complies with them
  • This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
  • New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit

@nyalldawson
Copy link
Collaborator

Looks good! Can you update the style test in test_provider_postgres to match?

@Gustry
Copy link
Contributor Author

Gustry commented Jun 8, 2018

@nyalldawson I just added some tests.

While I was writing the test, I realized that I always have a default style in my DB for my layers. So this patch works only when we are sure that there is a default style. I can add a function later to know if there is a default style. Is-it fine like this @nyalldawson ?

BTW, are PG tests running on travis? or only locally?

@m-kuhn
Copy link
Member

m-kuhn commented Jun 9, 2018

BTW, are PG tests running on travis? or only locally?

On travis also

@Gustry
Copy link
Contributor Author

Gustry commented Jun 10, 2018

OK. Let me know if it's OK for 3.2 or 3.4.

I will have some time in July about styles.

@nyalldawson nyalldawson added the API API improvement only, no visible user interface changes label Jun 10, 2018
@nyalldawson
Copy link
Collaborator

While I was writing the test, I realized that I always have a default style in my DB for my layers. So this patch works only when we are sure that there is a default style. I can add a function later to know if there is a default style. Is-it fine like this @nyalldawson ?

In that case, do you think it's better to just add a new function to retrieve the default style? (returning an empty string if none is set)

@Gustry
Copy link
Contributor Author

Gustry commented Jun 14, 2018

Yes, I'm planning to add this function as soon as the feature freeze is finished.
I would still need this PR anyway I think. If I implement only the get defaultStyle, I would need to compare each XML one per one to know which one is the default in the result from iface.activeLayer().listStylesInDatabase().

@Gustry
Copy link
Contributor Author

Gustry commented Jun 20, 2018

Gentle ping to have one PR less in the pending queue for 3.2?
I would still need this PR anyway for my side.

@nyalldawson
Copy link
Collaborator

Sure, let's merge.

@nyalldawson nyalldawson merged commit 6eeaca5 into qgis:master Jun 21, 2018
@Gustry
Copy link
Contributor Author

Gustry commented Jun 21, 2018

Thanks

@Gustry Gustry deleted the order_default_style branch June 21, 2018 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API improvement only, no visible user interface changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants