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

Fix QgsError.isEmpty() documentation #60488

Closed
wants to merge 1 commit into from

Conversation

kannes
Copy link
Contributor

@kannes kannes commented Feb 6, 2025

The method checks if there is an empty message list. It will actually return TRUE if there is no error message and FALSE if there is an error.


I was surprised when reading the documentation, stating that isEmpty() returning TRUE would indicate an error while intuition says "empty error = no error". Checking the code, it is indeed checking for error messages and if there are none, returns TRUE.

So the documentation was falsely inverted.

This PR simply fixes the documentation. I am certain that the behaviour must be kept to avoid lots of breakage everywhere.

The method checks if there is an empty message list. It will actually return `TRUE` if there is *no* error message and `FALSE` if there *is* an error.
@github-actions github-actions bot added this to the 3.42.0 milestone Feb 6, 2025
Copy link

github-actions bot commented Feb 6, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 71f14dd)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 71f14dd)

@kannes
Copy link
Contributor Author

kannes commented Feb 6, 2025

I currently haven't got a development setup so can't easily do the SIP stuff. I edited directly via GitHub. Sorry!

If someone else can and wants to push to this branch, please feel welcome to. I also would not mind a bit if someone simply made a new PR on their own.

@nyalldawson
Copy link
Collaborator

Superseded by #60503

@nyalldawson nyalldawson closed this Feb 7, 2025
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.

2 participants