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.

Sign up for GitHub

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

Jump to bottom

fix: Data Source Manager doesn't release CSV file after closing dialog #60663

Merged
merged 2 commits into from
Feb 20, 2025
Merged

fix: Data Source Manager doesn't release CSV file after closing dialog #60663

merged 2 commits into from
Feb 20, 2025

Conversation

hxbb00
Copy link
Contributor

@hxbb00 hxbb00 commented Feb 19, 2025

fix: #60649

@github-actions github-actions bot added this to the 3.42.0 milestone Feb 19, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 19, 2025 •
edited
Loading

🪟 Windows builds

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

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 6c979cc)

troopa81
troopa81 reviewed Feb 19, 2025
View reviewed changes
src/providers/delimitedtext/qgsdelimitedtextsourceselect.cpp Outdated
@@ -537,7 +537,7 @@ void QgsDelimitedTextSourceSelect::updateFieldLists()
// Run the scan in a separate thread
cancelScanTask();

mScanTask = new QgsDelimitedTextFileScanTask( url( /* skip overridden types */ true ) );
mScanTask = new QgsDelimitedTextFileScanTask( url( mFile.get(), /* skip overridden types */ true ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

there is an issue with clang-tidy, not sure why. Could you call clear() on mScanTask later in the code instead of affecting nullptr and see if it fixes the cland-tidy issue ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

call clear() do not fix it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/qgis/QGIS/actions/runs/13426309074/job/37512897078#step:9:1 it looks pass, but why has errors？

Copy link
Contributor

Choose a reason for hiding this comment

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

@ptitjano fixed a few things related to clang-tidy. Could you rebase your branch on current master

troopa81
troopa81 approved these changes Feb 20, 2025
View reviewed changes
Copy link
Contributor

@troopa81 troopa81 left a comment

Choose a reason for hiding this comment

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

All looks good, let's forget about clang-tidy here, this is not related to your modifications. We'll check this in another PR

@troopa81
Copy link
Contributor

All looks good, let's forget about clang-tidy here, this is not related to your modifications. We'll check this in another PR

forget what I just say, there is a clang-tidy issue. Not sure why

@troopa81
Copy link
Contributor

OK, here is the culprit llvm/llvm-project#58820

So, It has been fixed upstream. LGTM

@troopa81 troopa81 merged commit 31c4399 into qgis:master Feb 20, 2025
30 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@troopa81 troopa81 troopa81 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.42.0
Development

Successfully merging this pull request may close these issues.

Data Source Manager doesn't release CSV file after closing dialog
2 participants
@hxbb00 @troopa81