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

Add support for QML style from QGIS 3.28 #577

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

olsen232
Copy link
Contributor

@olsen232 olsen232 commented Dec 6, 2024

See #455

Fixes #455
Fixes #540
Fixes #541

Changes to conversion logic are basically the same as in previous attempt by geographika:
#569
it's only a very minor change to the conversion logic that is needed, so can't go too far wrong.

However, updated test data is quite different to that supplied by geographika - test data is mostly limited to the properties that we actually read and write (consistent with the old test data now in data/qmls_old) and not the complete output of QGIS, which tends to have a lot of noise that we can't parse and don't care about.

TESTED=tests pass, test data in both data/qmls and data/qmls_old can be understood by my QGIS-3.34 on macos.

@olsen232
Copy link
Contributor Author

olsen232 commented Dec 6, 2024

PS so far this PR only contains src/ and data/ changes.
Should I npm run build and commit compiled changes to dist/ also?

@olsen232
Copy link
Contributor Author

olsen232 commented Dec 9, 2024

@KaiVolland can you take a look?

@KaiVolland
Copy link
Contributor

PS so far this PR only contains src/ and data/ changes. Should I npm run build and commit compiled changes to dist/ also?

I'm actually not sure why dist is not in .gitignore. Usually this should be covered by the pipeline.

src/QGISStyleParser.ts Outdated Show resolved Hide resolved
@KaiVolland
Copy link
Contributor

KaiVolland commented Dec 11, 2024

Thanks for the contribution. 🙏 Just a minor note with a possible improvement.
You just need to adapt your commit messages. And don't care about the dist folder. :)

Tests should read both pre-3.28 QML - from data/qmls_old
and, post-3.28 QML - stored in data/qmls

When writing, the output should be 3.28 compatible.

This only sets up the new text structure -
the test data and conversion logic is not yet updated to be 3.28 compatible.
and to read both 3.28 style QML, and earlier style QML

Update test data in qmls/ to be 3.28 style QML
@KaiVolland KaiVolland merged commit 8c05cc8 into geostyler:main Dec 12, 2024
3 checks passed
@KaiVolland
Copy link
Contributor

Just FYI: I reworded your commit messages and force pushed to the main branch again. This is breaking change and it's my bad that i didn't notice.

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