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

[delimiter text] Add Point{Z,M,ZM} geometry support (fixes #25645) #31595

Merged
merged 2 commits into from
Sep 6, 2019

Conversation

nirvn
Copy link
Contributor

@nirvn nirvn commented Sep 6, 2019

Description

This PR implements Point{Z,M,ZM} geometry support to our delimiter text provider.
Screenshot from 2019-09-06 17-55-49

Checklist

  • Commit messages are descriptive and explain the rationale for changes
  • Commits which fix bugs include Fixes #11111 at the bottom of the commit message
  • I have read the QGIS Coding Standards and this PR complies with them
  • New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit
  • I have evaluated whether it is appropriate for this PR to be backported, backport requests are left as label or comment

@nirvn nirvn added Data Provider Related to specific vector, raster or mesh data providers Feature labels Sep 6, 2019
@nirvn nirvn added this to the 3.10.0 milestone Sep 6, 2019
@nirvn
Copy link
Contributor Author

nirvn commented Sep 6, 2019

@3nids , thanks for the quick review, that'll be my last feature pushed prior to freeze :)

isNull = false;
QgsPointXY pt;
bool ok = QgsDelimitedTextProvider::pointFromXY( sX, sY, pt, mSource->mDecimalPoint, mSource->mXyDms );
QgsPoint *pt = new QgsPoint();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
QgsPoint *pt = new QgsPoint();
std::unique_ptr<QgsPoint> pt = qgis::make_unique<QgsPoint>();

for good practice and so on ;)

@m-kuhn
Copy link
Member

m-kuhn commented Sep 6, 2019

I'm tempted to almost call it a fix

Copy link
Member

@m-kuhn m-kuhn left a comment

Choose a reason for hiding this comment

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

Looks good, feel free to merge and followup with RAII later on

@nirvn nirvn merged commit 5df3094 into qgis:master Sep 6, 2019
@nirvn
Copy link
Contributor Author

nirvn commented Sep 6, 2019

@m-kuhn , cheers, I'll follow up tomorrow when I get to my workstation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Data Provider Related to specific vector, raster or mesh data providers Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants